Incoherent animation of skinned meshes

full.metal.coder · Post by **full.metal.coder** » Thu Jul 24, 2008 9:39 pm

animation should be done in OnAnimate() and not in render(), even for skinned meshes...

here is a small fix (against SVN HEAD i.e r1420) : http://edyuk.org/misc/coherent-skinned- ... ation.diff

would be nice if it made it in next Irrlicht release.

hybrid · Post by **hybrid** » Thu Jul 24, 2008 10:02 pm

AFAIK, this won't work with more than one mesh node with the same mesh, but different animation. The OnAnimate changes will be overwritten by the following calls, and only the last OnAnimated will be rendered for all meshes.

full.metal.coder · Post by **full.metal.coder** » Thu Jul 24, 2008 10:59 pm

you've got a point there.

but then why doing anything at all in the OnAnimate() call since other meshes will also see their carefully computed animation data be overwritten in the exact same way...

I guess we might need a per-node mesh buffer to store animations then and thus avoid numerous computations while making it possible to consistently animate nodes and keep data available for physics (among other things).

full.metal.coder · Post by **full.metal.coder** » Fri Jul 25, 2008 10:19 am

Been working on that damn per-node storage this morning and I finally got it to work.
The patch spans over ISkinnedMesh.h, IAnimatedSceneNode.h and their respective implementations (i.e. CAnimatedMeshSceneNode.(h|cpp) and CSkinnedMesh.(h|cpp))

I've successfully tested it with both skin cache enabled and disabled and I made sure that the new two API functions were simple to use and followed Irrlicht naming conventions.

using this new feature is as simple as that :

Code: Select all

animatedMeshSceneNode->setUseSkinCache(true);

you can then access the data (kept up to date every OnAnimate() call

Code: Select all

irr::core::array<SSkinMeshBuffer*> *d = animatedMeshSceneNode->getSkinCache();

I also took this opportunity to improve one internal in skinned mesh : building of local animation matrices is not done too much times as it used to... (for n joints these matrices used to be built n times each, they are now build 1 time each...)

http://edyuk.org/misc/skin-cache.diff

If any of the maintainer could take care of this I'd be grateful for it.

rogerborg · Post by **rogerborg** » Fri Jul 25, 2008 10:28 am

Wow, that's meaty. Many thanks for looking into this!

Could you make your test code and resources available as well?

full.metal.coder · Post by **full.metal.coder** » Fri Jul 25, 2008 12:38 pm

My test code and resources are basically the game engine I'm working on for The First King project.

Everything is available on the Subversion repository : http://first-king.svn.sf.net/svnroot/first-king/trunk/

full.metal.coder · Post by **full.metal.coder** » Sat Jul 26, 2008 8:18 am

Is there any chance my code make it into Irrlicht SVN repo or will I be forced to ship a custom build of Irrlicht?

rogerborg · Post by **rogerborg** » Sat Jul 26, 2008 9:28 am

There's always a chance...

Have you done much work in a reviewed environment? Providing unit and integration tests demonstrating coverage of common, uncommon and edge cases goes a long way towards building confidence.

full.metal.coder · Post by **full.metal.coder** » Sat Jul 26, 2008 10:29 am

Depends... What do you consider as a "reviewed environment" ?

As for testing here are my three cents :

* Even it did not do everything I expect from it this patch is HARMLESS. Skin cache can be turned on/off on demand and when turned off everything works as if there was not patch
* The skin cache has no effect on non-skinned meshes
* I have tested this patch with a number of meshes (b3d, x, ms3d) in the project I'm working on and noticed no problem
* The skin cache actually does what it is expected to

The only possible improvement to it I can think about is making it work with non-skinned meshes but I have no interest for this and I doubt anyone has so...

I don't quite see how I could build confidence. I'm providing this patch because it is useful (and required by the game engine I'm working on). If by confidence you mean you want to ensure I'm not a noob throwing away a piece of crappy code then here's my answer :

I'm quite new to 3d and game programming but I've been following Irrlicht develoment for three years (since I started hacking C++) and worked on large C++ projects for two years. I invite you to take a look at Edyuk and QCodeEdit.

Of course it does not imply confidence but I'd say my patch deserves a quick look and this ought to convince you (or anyone else that might be in charge...)

rogerborg · Post by **rogerborg** » Sat Jul 26, 2008 12:50 pm

I'm not one of the devs, I'm just interested in this being committed, since it's a step towards enabling a collision patch that I've had backed up for ages.

The less work that the devs have to do to verify correct functionality, the more likely it is that this will get committed. At a quick glance, there are some warnings, sign differences, debug code left in, stylistic issues. Probably nothing major. I'll take a look myself and see if I can provide some verification.

You're testing it with the ninja.b3d model from your SVN repository, right?

Thanks again for submitting this; let's make sure that it gets in.

rogerborg · Post by **rogerborg** » Sat Jul 26, 2008 3:11 pm

OK, here's a cleaned up patch against SVN 1422 that removes the debug code and fixes use of an uninitialised variable and a memory leak (the SkinnedMeshBuffers needed dropped).

http://www.colinmacdonald.pwp.blueyonder.co.uk/skin-cache-1.diff

Test app: the expected behaviour is that each animation should proceed independently, and that no memory leaks should be reported on exit.

Actually, that's not entirely true; there are leaks from the B3D loader: I've put a separate patch up for that, and tested with both patched applied.

Code: Select all

#include <irrlicht.h>

using namespace irr;
using namespace core;
using namespace scene;
using namespace video;
using namespace io;
using namespace gui;

#ifdef _IRR_WINDOWS_
#pragma comment(lib, "Irrlicht.lib")
#endif

int main()
{
	IrrlichtDevice *device =
		createDevice( video::EDT_OPENGL, dimension2d<s32>(640, 480), 32,
			false, false, false, 0);

	IVideoDriver* driver = device->getVideoDriver();
	ISceneManager* smgr = device->getSceneManager();

	// https://first-king.svn.sf.net/svnroot/first-king/trunk/media/ninja.b3d
	IAnimatedMesh* ninjaMesh = smgr->getMesh("../../media/ninja.b3d");
	u32 const ninjaFrameCount = ninjaMesh->getFrameCount();    

	IAnimatedMesh* sydneyMesh = smgr->getMesh("../../media/sydney.md2");
	u32 const sydneyFrameCount = sydneyMesh->getFrameCount();

	for(int i = 0; i < 4; ++i)
	{
		IAnimatedMeshSceneNode* node = smgr->addAnimatedMeshSceneNode( ninjaMesh );
		node->setPosition(vector3df(5.f * i - 7.5f, 0, 0));
		node->setMaterialFlag(EMF_LIGHTING, false);
		node->setAnimationSpeed(5);
		node->setCurrentFrame((f32)ninjaFrameCount * i / 4);
		node->setUseSkinCache(i < 2);

		node = smgr->addAnimatedMeshSceneNode( sydneyMesh );
		node->setPosition(vector3df(5.f * i - 7.5f, 15, 0));
		node->setMaterialFlag(EMF_LIGHTING, false);
		node->setAnimationSpeed(5);
		node->setCurrentFrame((f32)sydneyFrameCount * i / 4);
		node->setUseSkinCache(i < 2);
		node->setScale(vector3df(0.1f, 0.1f, 0.1f));
	}
   
	ICameraSceneNode * camera = smgr->addCameraSceneNodeFPS(0, 50, 50);
	camera->setPosition(vector3df(0, 10, -15));

	while(device->run())
	{
		driver->beginScene(true, true, SColor(255,100,101,140));
		smgr->drawAll();
		driver->endScene();
	}

	device->drop();
	return 0;
}

christianclavet · Post by **christianclavet** » Sat Jul 26, 2008 4:23 pm

Wow! Thanks for your help Rogerborg! And you too FullMetalCoder, I was not aware that the problem was that deep! Also thanks for the link to the STYLE link, I've got something to refer to now!

full.metal.coder · Post by **full.metal.coder** » Sat Jul 26, 2008 4:27 pm

Great! Very kind of you to take the time to clean things up.

Let's hope a dev will come and pick it up. Btw any idea when next Irrlicht release is due to drop? There hasn't been much news from the devs lately...

hybrid · Post by **hybrid** » Sat Jul 26, 2008 9:17 pm

Uhh? I post an answer to the next release date almost every day. Anyway, it'll be sometimes later this year. Nothing fixed, yet.
The problem with many patches (I did not check this ine, so this is a general observation) is that they add new features in a way that does not fit the existing approaches. So it takes lots of time to adapt the new functionality to the Irrlicht way of doing things. And this takes most of the time, besides testing of course.
But I guess that Luke might take up this one, or someone else once we have some time for it.

full.metal.coder · Post by **full.metal.coder** » Sun Jul 27, 2008 10:15 am

Nevermind the release question. I'd be content with the patch being committed to the SVN for now.

If you have any concern about this patch "not fitting the Irrlicht way of doing things" please voice them or explain what is the Irrlicht way of doing things so that we may check that the patch does not break them and adapt it if necessary. You see I'm willing to work on this issue as much as needed to get this patch integrated so it's not as if I were asking for a feature and waiting for the devs to do all the work (when someone asked me to bring D support to the C++ IDE I've been working on for more than two years without giving any help it felt... odd to say the least)

Irrlicht Engine

Incoherent animation of skinned meshes

Incoherent animation of skinned meshes

[PATCH] incoherent animation of skinned meshes