Merge branch 'keyframe' into 'master'

Encapsulate NIF transformations, round 2 (#6709)

Closes #6709

See merge request OpenMW/openmw!1989

CHANGELOG.md

@@ -169,6 +169,7 @@
     Task #6264: Remove the old classes in animation.cpp
     Task #6553: Simplify interpreter instruction registration
     Task #6564: Remove predefined data paths `data="?global?data"`, `data=./data`
+    Task #6709: Move KeyframeController transformation magic to NifOsg::MatrixTransform
 
 0.47.0
 ------

components/nifosg/controller.cpp

@@ -165,48 +165,17 @@ void KeyframeController::operator() (NifOsg::MatrixTransform* node, osg::NodeVis
 {
     if (hasInput())
     {
-        osg::Matrix mat = node->getMatrix();
-
         float time = getInputValue(nv);
 
-        Nif::Matrix3& rot = node->mRotationScale;
+        if (!mRotations.empty())
-
+            node->setRotation(mRotations.interpKey(time));
-        bool setRot = false;
-        if(!mRotations.empty())
-        {
-            mat.setRotate(mRotations.interpKey(time));
-            setRot = true;
-        }
         else if (!mXRotations.empty() || !mYRotations.empty() || !mZRotations.empty())
-        {
+            node->setRotation(getXYZRotation(time));
-            mat.setRotate(getXYZRotation(time));
-            setRot = true;
-        }
-        else
-        {
-            // no rotation specified, use the previous value
-            for (int i=0;i<3;++i)
-                for (int j=0;j<3;++j)
-                    mat(j,i) = rot.mValues[i][j]; // NB column/row major difference
-        }
 
-        if (setRot) // copy the new values back
+        if (!mScales.empty())
-            for (int i=0;i<3;++i)
+            node->setScale(mScales.interpKey(time));
-                for (int j=0;j<3;++j)
+        if (!mTranslations.empty())
-                    rot.mValues[i][j] = mat(j,i); // NB column/row major difference
+            node->setTranslation(mTranslations.interpKey(time));
-
-        float& scale = node->mScale;
-        if(!mScales.empty())
-            scale = mScales.interpKey(time);
-
-        for (int i=0;i<3;++i)
-            for (int j=0;j<3;++j)
-                mat(i,j) *= scale;
-
-        if(!mTranslations.empty())
-            mat.setTrans(mTranslations.interpKey(time));
-
-        node->setMatrix(mat);
     }
 
     traverse(node, nv);
@@ -396,14 +365,16 @@ void RollController::operator() (osg::MatrixTransform* node, osg::NodeVisitor* n
         mStartingTime = newTime;
 
         float value = mData.interpKey(getInputValue(nv));
-        osg::Matrix matrix = node->getMatrix();
 
         // Rotate around "roll" axis.
         // Note: in original game rotation speed is the framerate-dependent in a very tricky way.
         // Do not replicate this behaviour until we will really need it.
         // For now consider controller's current value as an angular speed in radians per 1/60 seconds.
-        matrix = osg::Matrix::rotate(value * duration * 60.f, 0, 0, 1) * matrix;
+        node->preMult(osg::Matrix::rotate(value * duration * 60.f, 0, 0, 1));
-        node->setMatrix(matrix);
+
+        // Note: doing it like this means RollControllers are not compatible with KeyframeControllers.
+        // KeyframeController currently wins the conflict.
+        // However unlikely that is, NetImmerse might combine the transformations somehow.
     }
 }
 
@@ -590,7 +561,7 @@ void ParticleSystemController::operator() (osgParticle::ParticleProcessor* node,
 }
 
 PathController::PathController(const PathController &copy, const osg::CopyOp &copyop)
-    : SceneUtil::NodeCallback<PathController, osg::MatrixTransform*>(copy, copyop)
+    : SceneUtil::NodeCallback<PathController, NifOsg::MatrixTransform*>(copy, copyop)
     , Controller(copy)
     , mPath(copy.mPath)
     , mPercent(copy.mPercent)
@@ -615,7 +586,7 @@ float PathController::getPercent(float time) const
     return percent;
 }
 
-void PathController::operator() (osg::MatrixTransform* node, osg::NodeVisitor* nv)
+void PathController::operator() (NifOsg::MatrixTransform* node, osg::NodeVisitor* nv)
 {
     if (mPath.empty() || mPercent.empty() || !hasInput())
     {
@@ -623,13 +594,9 @@ void PathController::operator() (osg::MatrixTransform* node, osg::NodeVisitor* n
         return;
     }
 
-    osg::Matrix mat = node->getMatrix();
-
     float time = getInputValue(nv);
     float percent = getPercent(time);
-    osg::Vec3f pos(mPath.interpKey(percent));
+    node->setTranslation(mPath.interpKey(percent));
-    mat.setTrans(pos);
-    node->setMatrix(mat);
 
     traverse(node, nv);
 }

components/nifosg/controller.hpp

@@ -397,7 +397,7 @@ namespace NifOsg
         float mEmitStop;
     };
 
-    class PathController : public SceneUtil::NodeCallback<PathController, osg::MatrixTransform*>, public SceneUtil::Controller
+    class PathController : public SceneUtil::NodeCallback<PathController, NifOsg::MatrixTransform*>, public SceneUtil::Controller
     {
     public:
         PathController(const Nif::NiPathController* ctrl);
@@ -406,7 +406,7 @@ namespace NifOsg
 
         META_Object(NifOsg, PathController)
 
-        void operator() (osg::MatrixTransform*, osg::NodeVisitor*);
+        void operator() (NifOsg::MatrixTransform*, osg::NodeVisitor*);
 
     private:
         Vec3Interpolator mPath;

components/nifosg/matrixtransform.cpp

@@ -2,11 +2,6 @@
 
 namespace NifOsg
 {
-    MatrixTransform::MatrixTransform()
-        : osg::MatrixTransform()
-    {
-    }
-
     MatrixTransform::MatrixTransform(const Nif::Transformation &trafo)
         : osg::MatrixTransform(trafo.toMatrix())
         , mScale(trafo.scale)
@@ -20,4 +15,48 @@ namespace NifOsg
         , mRotationScale(copy.mRotationScale)
     {
     }
+
+    void MatrixTransform::setScale(float scale)
+    {
+        if (mScale == scale)
+            return;
+
+        // Rescale the node using the known decomposed rotation.
+        for (int i = 0; i < 3; ++i)
+            for (int j = 0; j < 3; ++j)
+                _matrix(i,j) = mRotationScale.mValues[j][i] * scale; // NB: column/row major difference
+
+        // Update the current decomposed scale.
+        mScale = scale;
+
+        _inverseDirty = true;
+        dirtyBound();
+    }
+
+    void MatrixTransform::setRotation(const osg::Quat &rotation)
+    {
+        // First override the rotation ignoring the scale.
+        _matrix.setRotate(rotation);
+        for (int i = 0; i < 3; ++i)
+        {
+            for (int j = 0; j < 3; ++j)
+            {
+                // Update the current decomposed rotation and restore the known scale.
+                mRotationScale.mValues[j][i] = _matrix(i,j); // NB: column/row major difference
+                _matrix(i,j) *= mScale;
+            }
+        }
+
+        _inverseDirty = true;
+        dirtyBound();
+    }
+
+    void MatrixTransform::setTranslation(const osg::Vec3f &translation)
+    {
+        // The translation is independent from the rotation and scale so we can apply it directly.
+        _matrix.setTrans(translation);
+
+        _inverseDirty = true;
+        dirtyBound();
+    }
 }

components/nifosg/matrixtransform.hpp

@@ -11,7 +11,7 @@ namespace NifOsg
     class MatrixTransform : public osg::MatrixTransform
     {
     public:
-        MatrixTransform();
+        MatrixTransform() = default;
         MatrixTransform(const Nif::Transformation &trafo);
         MatrixTransform(const MatrixTransform &copy, const osg::CopyOp &copyop);
 
@@ -24,6 +24,13 @@ namespace NifOsg
         // we store the scale and rotation components separately here.
         float mScale{0.f};
         Nif::Matrix3 mRotationScale;
+
+        // 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
+        // unless you're sure you know what you are doing.
+        void setScale(float scale);
+        void setRotation(const osg::Quat &rotation);
+        void setTranslation(const osg::Vec3f &translation);
     };
 
 }