Skip to content

Commit

Permalink
Merge branch 'fix-osga-rotate-wildly' into 'master'
Browse files Browse the repository at this point in the history
Fix OSGAnimation issues

See merge request OpenMW/openmw!3989
  • Loading branch information
AnyOldName3 committed Apr 20, 2024
2 parents e4c70b7 + b7aa3b9 commit 04f1dc2
Show file tree
Hide file tree
Showing 15 changed files with 312 additions and 65 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion apps/opencs/view/render/actor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ namespace CSVRender

osg::ref_ptr<osg::PositionAttitudeTransform> mBaseNode;
SceneUtil::Skeleton* mSkeleton;
SceneUtil::NodeMapVisitor::NodeMap mNodeMap;
SceneUtil::NodeMap mNodeMap;
};
}

Expand Down
18 changes: 16 additions & 2 deletions apps/openmw/mwrender/animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down Expand Up @@ -964,8 +965,17 @@ namespace MWRender
{
if (!mNodeMapCreated && mObjectRoot)
{
SceneUtil::NodeMapVisitor visitor(mNodeMap);
mObjectRoot->accept(visitor);
// 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;
Expand Down Expand Up @@ -1479,6 +1489,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);

Expand Down
1 change: 1 addition & 0 deletions apps/openmw/mwrender/animation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ namespace MWRender
osg::ref_ptr<SceneUtil::LightListCallback> mLightListCallback;

bool mPlayScriptedOnly;
bool mRequiresBoneMap;

const NodeMap& getNodeMap() const;

Expand Down
12 changes: 12 additions & 0 deletions apps/openmw/mwrender/rotatecontroller.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "rotatecontroller.hpp"

#include <osg/MatrixTransform>
#include <osgAnimation/Bone>

namespace MWRender
{
Expand Down Expand Up @@ -43,6 +44,17 @@ namespace MWRender

node->setMatrix(matrix);

// If we are linked to a bone we must call setMatrixInSkeletonSpace
osgAnimation::Bone* b = dynamic_cast<osgAnimation::Bone*>(node);
if (b)
{
osgAnimation::Bone* parent = b->getBoneParent();
if (parent)
matrix *= parent->getMatrixInSkeletonSpace();

b->setMatrixInSkeletonSpace(matrix);
}

traverse(node, nv);
}

Expand Down
2 changes: 2 additions & 0 deletions apps/openmw_test_suite/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ file(GLOB UNITTEST_SRC_FILES
resource/testobjectcache.cpp

vfs/testpathutil.cpp

sceneutil/osgacontroller.cpp
)

source_group(apps\\openmw_test_suite FILES openmw_test_suite.cpp ${UNITTEST_SRC_FILES})
Expand Down
131 changes: 131 additions & 0 deletions apps/openmw_test_suite/sceneutil/osgacontroller.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
#include <components/sceneutil/osgacontroller.hpp>

#include <gtest/gtest.h>
#include <osgAnimation/Channel>

#include <filesystem>
#include <fstream>

namespace
{
using namespace SceneUtil;

static const std::string ROOT_BONE_NAME = "bip01";

// Creates a merged anim track with a single root channel with two start/end matrix transforms
osg::ref_ptr<Resource::Animation> createMergedAnimationTrack(std::string name, osg::Matrixf startTransform,
osg::Matrixf endTransform, float startTime = 0.0f, float endTime = 1.0f)
{
osg::ref_ptr<Resource::Animation> mergedAnimationTrack = new Resource::Animation;
mergedAnimationTrack->setName(name);

osgAnimation::MatrixKeyframeContainer* cbCntr = new osgAnimation::MatrixKeyframeContainer;
cbCntr->push_back(osgAnimation::MatrixKeyframe(startTime, startTransform));
cbCntr->push_back(osgAnimation::MatrixKeyframe(endTime, endTransform));

osg::ref_ptr<osgAnimation::MatrixLinearChannel> rootChannel = new osgAnimation::MatrixLinearChannel;
rootChannel->setName("transform");
rootChannel->setTargetName(ROOT_BONE_NAME);
rootChannel->getOrCreateSampler()->setKeyframeContainer(cbCntr);
mergedAnimationTrack->addChannel(rootChannel);
return mergedAnimationTrack;
}

TEST(OsgAnimationControllerTest, getTranslationShouldReturnSampledChannelTranslationForBip01)
{
std::vector<EmulatedAnimation> emulatedAnimations;
emulatedAnimations.push_back({ 0.0f, 1.0f, "test1" }); // should sample this
emulatedAnimations.push_back({ 1.1f, 2.0f, "test2" }); // should ignore this

OsgAnimationController controller;
controller.setEmulatedAnimations(emulatedAnimations);

osg::Matrixf startTransform = osg::Matrixf::identity();
osg::Matrixf endTransform = osg::Matrixf::identity();
osg::Matrixf endTransform2 = osg::Matrixf::identity();
endTransform.setTrans(1.0f, 1.0f, 1.0f);
controller.addMergedAnimationTrack(createMergedAnimationTrack("test1", startTransform, endTransform));
endTransform2.setTrans(2.0f, 2.0f, 2.0f);
controller.addMergedAnimationTrack(
createMergedAnimationTrack("test2", endTransform, endTransform2, 0.1f, 0.9f));

// should be halfway between 0,0,0 and 1,1,1
osg::Vec3f translation = controller.getTranslation(0.5f);
EXPECT_EQ(translation, osg::Vec3f(0.5f, 0.5f, 0.5f));
}

TEST(OsgAnimationControllerTest, getTranslationShouldReturnZeroVectorIfNotFound)
{
std::vector<EmulatedAnimation> emulatedAnimations;
emulatedAnimations.push_back({ 0.0f, 1.0f, "test1" });

OsgAnimationController controller;
controller.setEmulatedAnimations(emulatedAnimations);

osg::Matrixf startTransform = osg::Matrixf::identity();
osg::Matrixf endTransform = osg::Matrixf::identity();
endTransform.setTrans(1.0f, 1.0f, 1.0f);
controller.addMergedAnimationTrack(createMergedAnimationTrack("test1", startTransform, endTransform));

// Has no emulated animation at time so will return 0,0,0
osg::Vec3f translation = controller.getTranslation(100.0f);
EXPECT_EQ(translation, osg::Vec3f(0.0f, 0.0f, 0.0f));
}

TEST(OsgAnimationControllerTest, getTranslationShouldReturnZeroVectorIfNoMergedTracks)
{
std::vector<EmulatedAnimation> emulatedAnimations;
emulatedAnimations.push_back({ 0.0f, 1.0f, "test1" });

OsgAnimationController controller;
controller.setEmulatedAnimations(emulatedAnimations);

// Has no merged tracks so will return 0,0,0
osg::Vec3f translation = controller.getTranslation(0.5);
EXPECT_EQ(translation, osg::Vec3f(0.0f, 0.0f, 0.0f));
}

TEST(OsgAnimationControllerTest, getTransformShouldReturnIdentityIfNotFound)
{
std::vector<EmulatedAnimation> emulatedAnimations;
emulatedAnimations.push_back({ 0.0f, 1.0f, "test1" });

OsgAnimationController controller;
controller.setEmulatedAnimations(emulatedAnimations);

osg::Matrixf startTransform = osg::Matrixf::identity();
osg::Matrixf endTransform = osg::Matrixf::identity();
endTransform.setTrans(1.0f, 1.0f, 1.0f);
controller.addMergedAnimationTrack(createMergedAnimationTrack("test1", startTransform, endTransform));

// Has no emulated animation at time so will return identity
EXPECT_EQ(controller.getTransformForNode(100.0f, ROOT_BONE_NAME), osg::Matrixf::identity());

// Has no bone animation at time so will return identity
EXPECT_EQ(controller.getTransformForNode(0.5f, "wrongbone"), osg::Matrixf::identity());
}

TEST(OsgAnimationControllerTest, getTransformShouldReturnSampledAnimMatrixAtTime)
{
std::vector<EmulatedAnimation> emulatedAnimations;
emulatedAnimations.push_back({ 0.0f, 1.0f, "test1" }); // should sample this
emulatedAnimations.push_back({ 1.1f, 2.0f, "test2" }); // should ignore this

OsgAnimationController controller;
controller.setEmulatedAnimations(emulatedAnimations);

osg::Matrixf startTransform = osg::Matrixf::identity();
osg::Matrixf endTransform = osg::Matrixf::identity();
endTransform.setTrans(1.0f, 1.0f, 1.0f);
controller.addMergedAnimationTrack(createMergedAnimationTrack("test1", startTransform, endTransform));
osg::Matrixf endTransform2 = osg::Matrixf::identity();
endTransform2.setTrans(2.0f, 2.0f, 2.0f);
controller.addMergedAnimationTrack(
createMergedAnimationTrack("test2", endTransform, endTransform2, 0.1f, 0.9f));

EXPECT_EQ(controller.getTransformForNode(0.0f, ROOT_BONE_NAME), startTransform); // start of test1
EXPECT_EQ(controller.getTransformForNode(1.0f, ROOT_BONE_NAME), endTransform); // end of test1
EXPECT_EQ(controller.getTransformForNode(1.1f, ROOT_BONE_NAME), endTransform); // start of test2
EXPECT_EQ(controller.getTransformForNode(2.0f, ROOT_BONE_NAME), endTransform2); // end of test2
}
}
7 changes: 7 additions & 0 deletions components/misc/strings/algorithm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
25 changes: 14 additions & 11 deletions components/resource/keyframemanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
"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_middle2", "bip01_l_middle3", "bip01_l_pointer1", "bip01_l_pointer2", "bip01_l_pointer3",
"bip01_l_thumb1", "bip01_l_thumb2", "bip01_l_thumb3", "left_forearm" };
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 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 thumb1", "bip01 l thumb2", "bip01 l thumb3", "left forearm" };

if (std::find(boneNames.begin(), boneNames.end(), name) != boneNames.end())
return true;
Expand All @@ -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",
"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_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" };
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",
"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 pointer1", "bip01 r pointer2", "bip01 r pointer3", "right forearm" };

if (std::find(boneNames.begin(), boneNames.end(), name) != boneNames.end())
return true;
Expand All @@ -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;
Expand Down Expand Up @@ -99,6 +99,9 @@ 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()))
Expand Down
Loading

0 comments on commit 04f1dc2

Please sign in to comment.