Grammar cleanup, code cleanup, reduce logging, revert mRotation change

apps/openmw/mwrender/animation.cpp

@@ -416,7 +416,7 @@ namespace MWRender
             // guaranteed to update before
             osg::ref_ptr<osg::Callback> cb = new BoneAnimBlendControllerWrapper(controller, bone);
 
-            // Ensure there is no other AnimBlendController - this can happen if using
+            // Ensure there is no other AnimBlendController - this can happen when using
             // multiple animations with different roots, such as NPC animation
             osg::Callback* updateCb = bone->getUpdateCallback();
             while (updateCb)
@@ -434,7 +434,7 @@ namespace MWRender
             }
 
             // Find UpdateBone callback and bind to just after that (order is important)
-            // NOTE: if it doesnt have an UpdateBone callback, we shouldnt be doing blending!
+            // NOTE: if it doesn't have an UpdateBone callback, we shouldn't be doing blending!
             updateCb = bone->getUpdateCallback();
             while (updateCb)
             {
@@ -666,7 +666,6 @@ namespace MWRender
 
         for (const auto& name : mResourceSystem->getVFS()->getRecursiveDirectoryIterator(animationPath))
         {
-
             if (Misc::getFileExtension(name) == "kf")
             {
                 addSingleAnimSource(name, baseModel);
@@ -769,13 +768,22 @@ namespace MWRender
             Misc::StringUtils::replaceLast(yamlpath, ".dae", ".yaml");
 
             // globalBlendConfigPath is only used with actors! Objects have no default blending.
-            std::string_view globalBlendConfigPath = "animations/animation-config.yaml";
+            const VFS::Path::NormalizedView globalBlendConfigPath("animations/animation-config.yaml");
+            const VFS::Path::NormalizedView blendConfigPath(yamlpath);
 
             osg::ref_ptr<const SceneUtil::AnimBlendRules> blendRules;
             if (mPtr.getClass().isActor())
-                blendRules = mResourceSystem->getAnimBlendRulesManager()->getRules(globalBlendConfigPath, yamlpath);
+            {
+                blendRules
+                    = mResourceSystem->getAnimBlendRulesManager()->getRules(globalBlendConfigPath, blendConfigPath);
+                if (blendRules == nullptr)
+                    Log(Debug::Warning) << "Animation blending files were not found '" << blendConfigPath.value()
+                                        << "' or '" << globalBlendConfigPath.value() << "'";
+            }
             else
-                blendRules = mResourceSystem->getAnimBlendRulesManager()->getRules(yamlpath);
+            {
+                blendRules = mResourceSystem->getAnimBlendRulesManager()->getRules(blendConfigPath);
+            }
 
             // At this point blendRules will either be nullptr or an AnimBlendRules instance with > 0 rules inside.
             animsrc->mAnimBlendRules = blendRules;
@@ -1076,6 +1084,48 @@ namespace MWRender
         return mNodeMap;
     }
 
+    template <typename ControllerType, typename NodeType>
+    inline osg::Callback* Animation::handleBlendTransform(osg::ref_ptr<osg::Node> node,
+        osg::ref_ptr<SceneUtil::KeyframeController> keyframeController,
+        std::map<osg::ref_ptr<osg::Node>, osg::ref_ptr<AnimBlendControllerBase<NodeType>>>& blendControllers,
+        const AnimBlendStateData& stateData, const osg::ref_ptr<const SceneUtil::AnimBlendRules>& blendRules,
+        const AnimState& active)
+    {
+        osg::ref_ptr<ControllerType> animController;
+
+        if (blendControllers.contains(node))
+        {
+            animController = blendControllers[node];
+            animController->setKeyframeTrack(keyframeController, stateData, blendRules);
+        }
+        else
+        {
+            animController = new ControllerType(keyframeController, stateData, blendRules);
+            blendControllers[node] = animController;
+
+            if constexpr (std::is_same_v<ControllerType, BoneAnimBlendController>)
+                assignBoneBlendCallbackRecursive(animController, mActiveControllers, node, true);
+        }
+
+        keyframeController->mTime = active.mTime;
+
+        if constexpr (std::is_same_v<ControllerType, BoneAnimBlendController>)
+        {
+            // IMPORTANT: we must gather all transforms at point of change before next update
+            // instead of at the root update callback because the root bone may require blending.
+            if (animController->getBlendTrigger())
+                animController->gatherRecursiveBoneTransforms(static_cast<osgAnimation::Bone*>(node.get()));
+
+            // Register blend callback after the initial animation callback
+            node->addUpdateCallback(animController->getAsCallback());
+            mActiveControllers.emplace_back(node, animController->getAsCallback());
+
+            return keyframeController->getAsCallback();
+        }
+
+        return animController->getAsCallback();
+    }
+
     void Animation::resetActiveGroups()
     {
         // remove all previous external controllers from the scene graph
@@ -1122,70 +1172,24 @@ namespace MWRender
                     osg::ref_ptr<osg::Node> node = getNodeMap().at(
                         it->first); // this should not throw, we already checked for the node existing in addAnimSource
 
-                    osg::Callback* callback;
                     const bool useSmoothAnims = Settings::game().mSmoothAnimTransitions;
-                    if (useSmoothAnims && dynamic_cast<NifOsg::MatrixTransform*>(node.get()))
+                    const bool isNifTransform = dynamic_cast<NifOsg::MatrixTransform*>(node.get()) != nullptr;
+                    const bool isBoneTransform = dynamic_cast<osgAnimation::Bone*>(node.get()) != nullptr;
+
+                    osg::Callback* callback = it->second->getAsCallback();
+                    if (useSmoothAnims)
                     {
-                        // Update an existing animation blending controller or create a new one for NIF animations
+                        if (isNifTransform)
-                        osg::ref_ptr<AnimBlendController> animController;
-
-                        if (mAnimBlendControllers.contains(node))
                         {
-                            animController = mAnimBlendControllers[node];
+                            callback = handleBlendTransform<AnimBlendController, NifOsg::MatrixTransform>(node,
-                            animController->setKeyframeTrack(it->second, stateData, animsrc->mAnimBlendRules);
+                                it->second, mAnimBlendControllers, stateData, animsrc->mAnimBlendRules, active->second);
                         }
-                        else
+                        else if (isBoneTransform)
                         {
-                            animController = osg::ref_ptr<AnimBlendController>(
+                            callback
-                                new AnimBlendController(it->second, stateData, animsrc->mAnimBlendRules));
+                                = handleBlendTransform<BoneAnimBlendController, osgAnimation::Bone>(node, it->second,
-
+                                    mBoneAnimBlendControllers, stateData, animsrc->mAnimBlendRules, active->second);
-                            mAnimBlendControllers[node] = animController;
                         }
-
-                        it->second->mTime = active->second.mTime;
-
-                        callback = animController->getAsCallback();
-                    }
-                    else if (useSmoothAnims && dynamic_cast<osgAnimation::Bone*>(node.get()))
-                    {
-                        // Update an existing animation blending controller or create a new one for osgAnimation
-                        osg::ref_ptr<BoneAnimBlendController> animController;
-
-                        if (mBoneAnimBlendControllers.contains(node))
-                        {
-                            animController = mBoneAnimBlendControllers[node];
-                            animController->setKeyframeTrack(it->second, stateData, animsrc->mAnimBlendRules);
-                        }
-                        else
-                        {
-                            animController = osg::ref_ptr<BoneAnimBlendController>(
-                                new BoneAnimBlendController(it->second, stateData, animsrc->mAnimBlendRules));
-
-                            mBoneAnimBlendControllers[node] = animController;
-
-                            assignBoneBlendCallbackRecursive(animController, mActiveControllers, node, true);
-                        }
-
-                        // IMPORTANT: we must gather all transforms at point of change before next update
-                        // instead of at the root update callback because the root bone may need blending
-                        if (animController->getBlendTrigger())
-                            animController->gatherRecursiveBoneTransforms(static_cast<osgAnimation::Bone*>(node.get()));
-
-                        it->second->mTime = active->second.mTime;
-
-                        // Register blend callback after the initial animation callback
-                        callback = animController->getAsCallback();
-
-                        node->addUpdateCallback(callback);
-                        mActiveControllers.emplace_back(node, callback);
-
-                        // Ensure the original animation update callback is still applied
-                        // this is because we need this to happen first to get the latest transform to blend to
-                        callback = it->second->getAsCallback();
-                    }
-                    else
-                    {
-                        callback = it->second->getAsCallback();
                     }
 
                     node->addUpdateCallback(callback);

apps/openmw/mwrender/animation.hpp

@@ -51,7 +51,7 @@ namespace MWRender
     class RotateController;
     class TransparencyUpdater;
 
-    typedef std::vector<std::pair<osg::ref_ptr<osg::Node>, osg::ref_ptr<osg::Callback>>> ActiveControllersVector;
+    using ActiveControllersVector = std::vector<std::pair<osg::ref_ptr<osg::Node>, osg::ref_ptr<osg::Callback>>>;
 
     class EffectAnimationTime : public SceneUtil::ControllerSource
     {
@@ -312,6 +312,13 @@ namespace MWRender
 
         void removeFromSceneImpl();
 
+        template <typename ControllerType, typename NodeType>
+        inline osg::Callback* handleBlendTransform(osg::ref_ptr<osg::Node> node,
+            osg::ref_ptr<SceneUtil::KeyframeController> keyframeController,
+            std::map<osg::ref_ptr<osg::Node>, osg::ref_ptr<AnimBlendControllerBase<NodeType>>>& blendControllers,
+            const AnimBlendStateData& stateData, const osg::ref_ptr<const SceneUtil::AnimBlendRules>& blendRules,
+            const AnimState& active);
+
     public:
         Animation(
             const MWWorld::Ptr& ptr, osg::ref_ptr<osg::Group> parentNode, Resource::ResourceSystem* resourceSystem);

apps/openmw/mwrender/animblendcontroller.cpp

@@ -102,13 +102,10 @@ namespace MWRender
     void AnimBlendControllerBase<NodeClass>::setKeyframeTrack(osg::ref_ptr<SceneUtil::KeyframeController> kft,
         AnimBlendStateData newState, osg::ref_ptr<const SceneUtil::AnimBlendRules> blendRules)
     {
-        // If aimation has changed, start blending
+        // If animation has changed then start blending
         if (newState.mGroupname != mAnimState.mGroupname || newState.mStartKey != mAnimState.mStartKey
             || kft != mKeyframeTrack)
         {
-            // Allow logging of cahnge to aid with implementing animations for developers/modders
-            // Log(Debug::Verbose) << "Animation change to: " << newState.mGroupname << ":" << newState.mStartKey;
-
             // Default blend settings
             mBlendDuration = 0;
             mEasingFn = &Easings::sineOut;
@@ -167,12 +164,12 @@ namespace MWRender
 
         // Shouldnt happen, but potentially an edge case where a new bone was added
         // between gatherRecursiveBoneTransforms and this update
-        // currently OpenMW will never do this, but potentially useful
+        // currently OpenMW will never do this
         assert(mBlendBoneTransforms.find(bone) != mBlendBoneTransforms.end());
 
         // Every frame the osgAnimation controller updates this
         // so it is ok that we update it directly below
-        osg::Matrixf currentSampledMatrix = bone->getMatrix();
+        const osg::Matrixf& currentSampledMatrix = bone->getMatrix();
         const osg::Matrixf& lastSampledMatrix = mBlendBoneTransforms.at(bone);
 
         const osg::Vec3f scale = currentSampledMatrix.getScale();
@@ -262,7 +259,7 @@ namespace MWRender
             mBlendTrigger = false;
             mBlendStartTime = time;
             // Nif mRotation is used here because it's unaffected by the side-effects of RotationController
-            mBlendStartRot = node->mRotation.toOsgMatrix().getRotate();
+            mBlendStartRot = node->mRotationScale.toOsgMatrix().getRotate();
             mBlendStartTrans = node->getMatrix().getTrans();
             mBlendStartScale = node->mScale;
         }
@@ -280,7 +277,7 @@ namespace MWRender
             else
             {
                 // This is necessary to prevent first person animation glitching out
-                node->setRotation(node->mRotation);
+                node->setRotation(node->mRotationScale);
             }
 
             if (translation)
@@ -297,7 +294,7 @@ namespace MWRender
             if (rotation)
                 node->setRotation(*rotation);
             else
-                node->setRotation(node->mRotation);
+                node->setRotation(node->mRotationScale);
         }
 
         if (scale)

apps/openmw/mwrender/animblendcontroller.hpp

@@ -49,7 +49,7 @@ namespace MWRender
         void gatherRecursiveBoneTransforms(osgAnimation::Bone* parent, bool isRoot = true);
         void applyBoneBlend(osgAnimation::Bone* parent);
 
-        const char* libraryName() const override { return "openmw"; }
+        const char* libraryName() const override { return "MWRender"; }
         const char* className() const override { return "AnimBlendController"; }
 
     protected:
@@ -77,8 +77,8 @@ namespace MWRender
         std::unordered_map<osg::Node*, osg::Matrixf> mBlendBoneTransforms;
     };
 
-    typedef AnimBlendControllerBase<NifOsg::MatrixTransform> AnimBlendController;
+    using AnimBlendController = AnimBlendControllerBase<NifOsg::MatrixTransform>;
-    typedef AnimBlendControllerBase<osgAnimation::Bone> BoneAnimBlendController;
+    using BoneAnimBlendController = AnimBlendControllerBase<osgAnimation::Bone>;
 
     // Assigned to child bones with an instance of AnimBlendControllerBase
     class BoneAnimBlendControllerWrapper : public osg::Callback

components/nifosg/controller.cpp

@@ -186,7 +186,7 @@ namespace NifOsg
         else
         {
             // This is necessary to prevent first person animations glitching out due to RotationController
-            node->setRotation(node->mRotation);
+            node->setRotation(node->mRotationScale);
         }
 
         if (translation)

components/nifosg/matrixtransform.cpp

@@ -5,14 +5,14 @@ namespace NifOsg
     MatrixTransform::MatrixTransform(const Nif::NiTransform& transform)
         : osg::MatrixTransform(transform.toMatrix())
         , mScale(transform.mScale)
-        , mRotation(transform.mRotation)
+        , mRotationScale(transform.mRotation)
     {
     }
 
     MatrixTransform::MatrixTransform(const MatrixTransform& copy, const osg::CopyOp& copyop)
         : osg::MatrixTransform(copy, copyop)
         , mScale(copy.mScale)
-        , mRotation(copy.mRotation)
+        , mRotationScale(copy.mRotationScale)
     {
     }
 
@@ -24,7 +24,7 @@ namespace NifOsg
         // Rescale the node using the known components.
         for (int i = 0; i < 3; ++i)
             for (int j = 0; j < 3; ++j)
-                _matrix(i, j) = mRotation.mValues[j][i] * mScale; // NB: column/row major difference
+                _matrix(i, j) = mRotationScale.mValues[j][i] * mScale; // NB: column/row major difference
 
         _inverseDirty = true;
         dirtyBound();
@@ -40,7 +40,7 @@ namespace NifOsg
             for (int j = 0; j < 3; ++j)
             {
                 // Update the current decomposed rotation and restore the known scale.
-                mRotation.mValues[j][i] = _matrix(i, j); // NB: column/row major difference
+                mRotationScale.mValues[j][i] = _matrix(i, j); // NB: column/row major difference
                 _matrix(i, j) *= mScale;
             }
         }
@@ -52,12 +52,12 @@ namespace NifOsg
     void MatrixTransform::setRotation(const Nif::Matrix3& rotation)
     {
         // Update the decomposed rotation.
-        mRotation = rotation;
+        mRotationScale = rotation;
 
         // Reorient the node using the known components.
         for (int i = 0; i < 3; ++i)
             for (int j = 0; j < 3; ++j)
-                _matrix(i, j) = mRotation.mValues[j][i] * mScale; // NB: column/row major difference
+                _matrix(i, j) = mRotationScale.mValues[j][i] * mScale; // NB: column/row major difference
 
         _inverseDirty = true;
         dirtyBound();

components/nifosg/matrixtransform.hpp

@@ -23,8 +23,7 @@ namespace NifOsg
         // problems when a KeyframeController wants to change only one of these components. So
         // we store the scale and rotation components separately here.
         float mScale{ 0.f };
-
+        Nif::Matrix3 mRotationScale;
-        Nif::Matrix3 mRotation;
 
         // Utility methods to transform the node and keep these components up-to-date.
         // The matrix's components should not be overridden manually or using preMult/postMult

components/resource/animblendrulesmanager.cpp

@@ -31,7 +31,7 @@ namespace Resource
     }
 
     osg::ref_ptr<const AnimBlendRules> AnimBlendRulesManager::getRules(
-        std::string_view path, std::string_view overridePath)
+        const VFS::Path::NormalizedView path, const VFS::Path::NormalizedView overridePath)
     {
         // Note: Providing a non-existing path but an existing overridePath is not supported!
         auto tmpl = loadRules(path);
@@ -43,7 +43,7 @@ namespace Resource
         osg::ref_ptr<SceneUtil::AnimBlendRules> blendRules(new AnimBlendRules(*tmpl, osg::CopyOp::SHALLOW_COPY));
         blendRules->getOrCreateUserDataContainer()->addUserObject(new Resource::TemplateRef(tmpl));
 
-        if (!overridePath.empty())
+        if (!overridePath.value().empty())
         {
             auto blendRuleOverrides = loadRules(overridePath);
             if (blendRuleOverrides)
@@ -56,28 +56,28 @@ namespace Resource
         return blendRules;
     }
 
-    osg::ref_ptr<const AnimBlendRules> AnimBlendRulesManager::loadRules(std::string_view path)
+    osg::ref_ptr<const AnimBlendRules> AnimBlendRulesManager::loadRules(VFS::Path::NormalizedView path)
     {
-        const VFS::Path::Normalized normalizedPath(path);
+        const std::string normalized = VFS::Path::normalizeFilename(path.value());
-        std::optional<osg::ref_ptr<osg::Object>> obj = mCache->getRefFromObjectCacheOrNone(normalizedPath);
+        std::optional<osg::ref_ptr<osg::Object>> obj = mCache->getRefFromObjectCacheOrNone(normalized);
         if (obj.has_value())
         {
             return osg::ref_ptr<AnimBlendRules>(static_cast<AnimBlendRules*>(obj->get()));
         }
         else
         {
-            osg::ref_ptr<AnimBlendRules> blendRules = AnimBlendRules::fromFile(mVfs, normalizedPath);
+            osg::ref_ptr<AnimBlendRules> blendRules = AnimBlendRules::fromFile(mVfs, path);
             if (blendRules)
             {
                 // Blend rules were found in VFS, cache them.
-                mCache->addEntryToObjectCache(normalizedPath, blendRules);
+                mCache->addEntryToObjectCache(normalized, blendRules);
                 return blendRules;
             }
         }
 
         // No blend rules were found in VFS, cache a nullptr.
         osg::ref_ptr<AnimBlendRules> nullRules = nullptr;
-        mCache->addEntryToObjectCache(normalizedPath, nullRules);
+        mCache->addEntryToObjectCache(normalized, nullRules);
         return nullRules;
     }
 

components/resource/animblendrulesmanager.hpp

@@ -21,12 +21,12 @@ namespace Resource
         /// Retrieve a read-only keyframe resource by name (case-insensitive).
         /// @note Throws an exception if the resource is not found.
         osg::ref_ptr<const SceneUtil::AnimBlendRules> getRules(
-            std::string_view path, std::string_view overridePath = "");
+            const VFS::Path::NormalizedView path, const VFS::Path::NormalizedView overridePath = "");
 
         void reportStats(unsigned int frameNumber, osg::Stats* stats) const override;
 
     private:
-        osg::ref_ptr<const SceneUtil::AnimBlendRules> loadRules(std::string_view path);
+        osg::ref_ptr<const SceneUtil::AnimBlendRules> loadRules(VFS::Path::NormalizedView path);
 
         const VFS::Manager* mVfs;
     };

components/sceneutil/animblendrules.cpp

@@ -62,10 +62,7 @@ namespace SceneUtil
         Log(Debug::Debug) << "Attempting to load animation blending config '" << configPath << "'";
 
         if (!vfs->exists(configPath))
-        {
-            Log(Debug::Warning) << "Animation blending files was not found '" << configPath << "'";
             return nullptr;
-        }
 
         // Retrieving and parsing animation rules
         std::string rawYaml(std::istreambuf_iterator<char>(*vfs->get(configPath)), {});
@@ -147,22 +144,20 @@ namespace SceneUtil
         Misc::StringUtils::lowerCaseInPlace(toKey);
         for (auto rule = mRules.rbegin(); rule != mRules.rend(); ++rule)
         {
-            // TO DO: Also allow for partial wildcards at the end of groups and keys via std::string startswith method
             bool fromMatch = false;
             bool toMatch = false;
 
             // Pseudocode:
             // If not a wildcard and found a wildcard
             // starts with substr(0,wildcard)
-
             if (fitsRuleString(fromGroup, rule->mFromGroup)
-                && (fitsRuleString(fromKey, rule->mFromKey) || rule->mFromKey == ""))
+                && (rule->mFromKey.empty() || fitsRuleString(fromKey, rule->mFromKey)))
             {
                 fromMatch = true;
             }
 
             if ((fitsRuleString(toGroup, rule->mToGroup) || (rule->mToGroup == "$" && toGroup == fromGroup))
-                && (fitsRuleString(toKey, rule->mToKey) || rule->mToKey == ""))
+                && (rule->mToKey.empty() || fitsRuleString(toKey, rule->mToKey)))
             {
                 toMatch = true;
             }