Crash after attaching particle system to TagPoint

[claytongarrett]

I'm trying to attach a ParticleSystem to a TagPoint in our Ogre 2.1 project and after rendering the frame the first time I get the following crash:



Here's how I'm creating/attaching the particle system:

Code: Select all

Ogre::ParticleSystem* DotSceneLoader::processParticleSystem(TiXmlElement *XMLNode, SceneNode *parentNode, Item *parentItem)
{
    Ogre::Bone *bone = NULL;
    Ogre::ParticleSystem* ps = 0;
    
    // Process other attributes
    String boneName = getAttrib(XMLNode, "attach_bone");
    
    // see if we have a bone to process
    if(boneName != "")
    {
        // get a reference to the bone through the parent's skeleton
        Ogre::SkeletonInstance *skeletonInstance = parentItem->getSkeletonInstance();
        Ogre::IdString boneId = Ogre::IdString(boneName);
        bone = skeletonInstance->getBone(boneId);
    }
    
    // Process particle systems
    TiXmlElement *pElement = XMLNode->FirstChildElement("system");
    while(pElement)
    {
        std::string systemName = getAttrib(pElement, "name");
        ps = mSceneMgr->createParticleSystem(systemName);
        ps->setRenderQueueGroup(mDefaultRenderQueue+1);
        parentNode->setStatic(false);
        
        if(bone) {
                // get a tag point and attach it
                Ogre::TagPoint *tagPoint = mSceneMgr->createTagPoint();
                tagPoint->attachObject(ps);
                tagPoint->_getFullTransformUpdated();
            
                // add the tag point to the bone
                bone->addTagPoint(tagPoint);
        } else {
            parentNode->attachObject(ps);
        }
        
        ps->setVisible(true);
        
        pElement = pElement->NextSiblingElement("system");
    }
    
    return ps;
}

Is there something wrong with they way I'm attaching the particle system? For what it's worth, the code path where bone is nil works fine (there, i'm just attaching the particle system to a SceneNode).

Thanks for any help you can give!

[dark_sylinc]

Oh boy!, this was a combination of 3 different issues:

1. The TagPoint had an invalid mParent, causing a crash in mParent->_updateFromParent(). This was a bug. This is fixed.
2. TagPoint::updateFromParentImpl is not actually implemented, so you will now crash there, hitting an assert. You may disable the assert and see if it works ok for you. Trying to implement it properly should be possible, but it could be slow.
3. ParticleSystem::_executeTriggerEmitters got ported in a rush, therefore you'll see:

   Code: Select all

   //TODO: (dark_sylinc) Refactor this. ControllerManager gets executed before us
   //(because it doesn't know if we'll update a SceneNode)
   const Quaternion derivedOrientation( mParentNode->_getDerivedOrientationUpdated() );

The last point is the real heart of the problem, as it should be ideally calling mParentNode->_getDerivedOrientation(). Taking a look at SceneManager::updateSceneGraph; the call to ControllerManager::getSingleton().updateAllControllers(); should be done much later:

Code: Select all

// Update controllers 
    //ControllerManager::getSingleton().updateAllControllers(); --> It shouldn't be done here

    highLevelCull();
    _applySceneAnimations();
    updateAllTransforms();
    updateAllAnimations();
    updateAllTagPoints();
#ifdef OGRE_LEGACY_ANIMATIONS
    updateInstanceManagerAnimations();
#endif
    updateInstanceManagers();
    updateAllBounds( mEntitiesMemoryManagerUpdateList );
    updateAllBounds( mLightsMemoryManagerCulledList );
    
    ControllerManager::getSingleton().updateAllControllers(); //This is where it should be done

Moving the function (and changing mParentNode->_getDerivedOrientationUpdated --> mParentNode->_getDerivedOrientation) will fix the problem.
However I did not move updateAllControllers before because I couldn't take the time to analyze whether there were controllers that wanted to reposition/reorient/rescale nodes. If no controller does this, then it's safe to move it a few lines below (I'm afraid I'll have to use you as my guinea pig). If it's not safe to use it, you will later see asserts about mCachedTransformOutOfDate and similar.
Ideally those controllers could be split in two so that node-modifying controllers are ran before updating the scene graph, and those who don't run afterwards.

Alternatively you can try disabling the assert. My guess is that either everything still works ok, or the particles may lag one frame behind.

I'm sorry you had to find such a nasty combination of issues, this is rare.

Cheers
Matias

[claytongarrett]

Hey Matias! Thanks so much for the quick, detailed reply (and the source fix). I'll give this a shot and report back.

[claytongarrett]

I started implementing your fixes, and my issue worked after just fixing #1 and #2 (pulled your code in and removed the assert in TagPoint::updateFromParentImpl):

1) The TagPoint had an invalid mParent, causing a crash in mParent->_updateFromParent(). This was a bug. This is fixed.
2) TagPoint::updateFromParentImpl is not actually implemented, so you will now crash there, hitting an assert. You may disable the assert and see if it works ok for you. Trying to implement it properly should be possible, but it could be slow.

I then implemented your suggested changes to #3 and everything seems to be fine there too.

What's the best course of action going forward so I can keep up to date with the official Ogre source? I can submit a PR for the changes in #3. Is it ok to include the removed assert from #2 as well?

Thanks again for your quick help on this. Helps alot!

[dark_sylinc]

I then implemented your suggested changes to #3 and everything seems to be fine there too.

Yay! I suspected as much, but confirmation is better

What's the best course of action going forward so I can keep up to date with the official Ogre source? I can submit a PR for the changes in #3.

Is it ok to include the removed assert from #2 as well?

I'd like to keep it, so it's an exploding reminder that that section isn't done.

[claytongarrett]

So I actually think I need to attempt implementing TagPoint::updateFromParentImpl, if you think you could give me a basic outline of the steps necessary. I need it for another related part of our project.

I need to have an SceneNode that is a child of a TagPoint look at another SceneNode outside of my skeleton. In order to do that, it needs to know its derived position, which means I've got to implement TagPoint::updateFromParentImpl, correct?

If that's the case, can you give me the basic outline of the steps needed to code this correctly? Or is that something I'll have to piece together by digging around a bit?

Thanks for the help, Matias!

[dark_sylinc]

Ensuring proper update means going all to the top of the hierarchy.

TagPoints are children of Bones. Bones belong to skeleton that is attached to a node (could be a scene node, could be a tag point) and everything is ultimately child of the root node.

The tag point needs to at least update the node the skeleton is attached to, the skeleton and its bones, before updating itself.

I'm not sure if the pointers can be easily retrieved or if you'll need to add a new variable (hopefully you don't, to avoid making the bones fatter)

[claytongarrett]

The tag point needs to at least update the node the skeleton is attached to, the skeleton and its bones, before updating itself.

I get that we need to update the node the skeleton is attached to and the skeleton's bones, but what property of the skeleton itself needs to be updated? It doesn't have any transform that goes into the calculation right?

Also, from a bone, how do I get the Skeleton it's attached to?

[dark_sylinc]

Also, from a bone, how do I get the Skeleton it's attached to? I see this:

That's what I meant "I'm not sure if the pointers can be easily retrieved or if you'll need to add a new variable (hopefully you don't, to avoid making the bones fatter)"
A quick glance over OgreBone.h I don't see a direct way.
mGlobalIndex is only useful if you already have the SkeletonInstance pointer.

but what property of the skeleton itself needs to be updated? It doesn't have any transform that goes into the calculation right?

You don't have the guarantee that animations have been updated, so you need to update Skeleton animations so that bones are set in place.
So you'd need to:

1. Update the skeleton's animations
2. Update the node the skeleton is attached to
3. Update skeleton's bones (because the TagPoint inherits the bones)

Hence it could be slow (ok for a few tagpoints, but for many...).

[claytongarrett]

Ahhh, got it. Thanks!

[claytongarrett]

I'm knee deep in this implementation and in my testing, think I've found a bug. When I attach my TagPoint to my bone initially, eventually TagPoint::_setParentBone calls:

Code: Select all

this->migrateTo( nodeMemoryManager );

This winds up calling: Node::migrateTo which passes the TagPoint's mTransform to NodeMemoryManager::migrateTo. This method modifies the transform, replacing the parent Ogre/SceneManager/Dummy node with the Memory Manager's simple "Dummy Node" one. Not sure if that's a problem yet. But the bigger issue is that the transform had identity transforms in slots 1-3, but they get overridden/inited with all 0s here. This later causes problems when the 0s in the Transform's orientation cause its mOrientation to contain NaNs.

Here's an example of what changes where in TagPoint::_setParentBone:

Code: Select all

        NodeMemoryManager *nodeMemoryManager = &mCreator->_getTagPointNodeMemoryManager();

        // here, my data is good:
        // in console: p mTransform.mOrientation[0]
        // (Ogre::ArrayQuaternion) $0 = {
        //   mChunkBase = {
        //     [0] = (1, 1, 1, 1)
        //     [1] = (0, 0, 0, 0)
        //     [2] = (0, 0, 0, 0)
        //     [3] = (0, 0, 0, 0)
        //   }
        // }

        if( mNodeMemoryManager != nodeMemoryManager )
            this->migrateTo( nodeMemoryManager );

        // now, my data has been inited to 0s
        // in console: p mTransform.mOrientation[0]
        // (Ogre::ArrayQuaternion) $0 = {
        //   mChunkBase = {
        //     [0] = (1, 0, 0, 0)
        //     [1] = (0, 0, 0, 0)
        //     [2] = (0, 0, 0, 0)
        //     [3] = (0, 0, 0, 0)
        //   }
        // }

        // Somewhat hacky way to mark our base classes that we can't be attached.
        mParent = mCreator->getDummySceneNode();

      

Pardon me if any of my terminology or analysis is off. First time digging into the depths of Ogre and its memory management techniques. But I've learned a lot!

Any thoughts on a good way to tackle this so that it keeps the identity data (and maybe keeps the Scene Manager's dummy node as the parent if that's going to be important?)

Thanks!

[dark_sylinc]

Ok I see, this is a bug, as the NodeMemoryManager is not accounting for TagPoints which work a little different.

Edit: Mmm... debugging. The problem looks like something else.

[dark_sylinc]

I'm afraid I cannot reproduce your problem. Could you provide a way to reproduce your issue? (e.g. ideally modifying Sample_AnimationTagPoint)

I've analyzed the code and:

This winds up calling: Node::migrateTo which passes the TagPoint's mTransform to NodeMemoryManager::migrateTo. This method modifies the transform, replacing the parent Ogre/SceneManager/Dummy node with the Memory Manager's simple "Dummy Node" one.

This is intentional. The dummy node contains identity transforms.

But the bigger issue is that the transform had identity transforms in slots 1-3, but they get overridden/inited with all 0s here. This later causes problems when the 0s in the Transform's orientation cause its mOrientation to contain NaNs.

When you attach the tag point to the bone, its data gets transferred.
So before we do migrateTo, your data was this:

Code: Select all

// here, my data is good:
        // in console: p mTransform.mOrientation[0]
        // (Ogre::ArrayQuaternion) $0 = {
        //   mChunkBase = {
        //     [0] = ([b]1[/b], 1, 1, 1)
        //     [1] = ([b]0[/b], 0, 0, 0)
        //     [2] = ([b]0[/b], 0, 0, 0)
        //     [3] = ([b]0[/b], 0, 0, 0)
        //   }
        // }

The only thing that matters is the bolded numbers (the first column) as long as mTransform.mIndex = 0; if mTransform.mIndex == 1 then what matters is the second column, and if it's == 2, then it's the third column, and so on.

The other columns do not belong to your node.

After migration, you get:

Code: Select all

        // now, my data has been inited to 0s
        // in console: p mTransform.mOrientation[0]
        // (Ogre::ArrayQuaternion) $0 = {
        //   mChunkBase = {
        //     [0] = ([b]1[/b], 0, 0, 0)
        //     [1] = ([b]0[/b], 0, 0, 0)
        //     [2] = ([b]0[/b], 0, 0, 0)
        //     [3] = ([b]0[/b], 0, 0, 0)
        //   }
        // }

Your data has been successfully transferred (if you check mTransform.mOrientation, they're different pointers), assuming mTransform.mIndex == 0. The rest of the data is all 0s because they haven't been used yet.

So either your problem is somewhere else, or the mIndex != 0 in that case there is a bug. But I could not reproduce it.

[claytongarrett]

I realize that those 0s were not part of my node's transform, rather in the same block. But if I'm understanding things correctly, future calculations in Ogre with those 0s eventually become NaNs and cause problems. Let me add some further detail to try and explain where it becomes a problem.

When TagPoint::updateAllTransformsTagOnTag runs at my app startup after the TagPoint has been attached to the bone, this happens:

Code: Select all

void TagPoint::updateAllTransformsTagOnTag( const size_t numNodes, Transform t )
{

// ...

finalMat.streamToAoS( t.mDerivedTransform );

// our transform's block contains the 0s as mentioned in the previous post
// (lldb) p t.mDerivedOrientation[t.mIndex]
// (Ogre::ArrayQuaternion) $0 = {
//   mChunkBase = {
//     [0] = (1, 0, 0, 0)
//     [1] = (0, 0, 0, 0)
//     [2] = (0, 0, 0, 0)
//     [3] = (0, 0, 0, 0)
//   }
// }

finalMat.decomposition( *t.mDerivedPosition,
                                         *t.mDerivedScale,
                                         *t.mDerivedOrientation );

// now our transform's block contains NaN
// (lldb) p t.mDerivedOrientation[t.mIndex]
// (Ogre::ArrayQuaternion) $8 = {
//   mChunkBase = {
//     [0] = (0.869052588, NaN, NaN, NaN)
//     [1] = (-0.000622438849, NaN, NaN, NaN)
//     [2] = (-0.494718939, NaN, NaN, NaN)
//     [3] = (-0.000622438849, NaN, NaN, NaN)
//   }
// }

Those NaNs later cause this crash when calling Node::convertWorldToLocalPosition:

Assertion failed: (vmovemaskq_u32( vceqq_f32( arg1, arg1 ) ) == 0x0f && vmovemaskq_u32( vceqq_f32( arg2, arg2 ) ) == 0x0f && "Passing NaN values to CMov4"), function Cmov4, file /Users/claygarrett/Projects/****/ogre/OgreMain/include/Math/Array/NEON/Single/OgreMathlibNEON.h, line 273.



See the debug tree at the bottom for the NaNs.

If it helps, here's the hierarchy of objects that I have set up:

Code: Select all

RootSceneNode->
   SceneNode->
      Skeleton->
         Bones->
            TagPoint->
                SceneNode->
                   SceneNode
                   SceneNode
                   SceneNode

It would take me some time to set up some sample code to reproduce it. But I'll give that a shot if the above doesn't help narrow down the issue.

Thanks!!

[dark_sylinc]

Ahhh!! You should've started there!!!

It's convertWorldToLocalPosition what's crashing! No wonder I could not reproduce.

I see now the problem. In this particular case the assert is harmless, the code just performs operations using SIMD and then extracts the individual result we want; and the assert is telling us the result of the nodes we don't care is going to be incorrect because there's a routine that doesn't work well with NaNs.

While ignoring the assert is likely harmless, we try to avoid NaNs because: a. They can cause unnecessary slowdowns, b. They make looking for errors harder.

Simply speaking, this is a problem of the memory being initialized to zero, when it should probably have a different default and needs to be fixed.

[dark_sylinc]

Done.

Please test it and let me know how it goes.

[claytongarrett]

Ahhh!! You should've started there!!!

Haha! I thought about it but didn't know how much info would be helpful and didn't want to overload the post. But yeah, I'll start with the actual crash next time

Thanks a million for the fix. I'll give it a test!

Clay

[claytongarrett]

Hey Matias! I got pulled off on some other stuff, and now I'm back onto trying to implement TagPoint::updateFromParentImpl. I'm going to hack certain parts just to make sure the basic approach works, then go back and clean up things after the fact. So, first, I wanted to make sure that I'm thinking about this right. Your suggestions were:

1) Update the skeleton's animations

So I'm calling sceneManager->updateAllAnimations();. This is part of the hacky part for now I only have a couple of animations in my project.

2) Update the node the skeleton is attached to

So I'm calling mySkeletonsNode->_getFullTransformUpdated();. Also, hacky.

3) Update skeleton's bones (because the TagPoint inherits the bones)

So I'm calling skeleton->update(); Last hack.

Hopefully that puts my in the state that I can then start the chain of parent updates from my TagPoint (if not, let me know what i've got wrong). Now, where my first problem comes in: one of the first things I'm doing in TagPoint::UpdateFromParentImpl is this:

Code: Select all

mParentBone->_updateFromParent();
const Matrix4 &parentTransform = mParentBone->_getDerivedTransform();

The 2nd line calls

Code: Select all

parentNodeTransform.concatenateAffine( localSpaceBone );

This crashes with an assert:

Code: Select all

assert(isAffine() && m2.isAffine());

Here are the values of the 2 matrices.

Code: Select all

m2:
[0]	Ogre::Real	0.000988470739
[1]	Ogre::Real	-0.00101091934
[2]	Ogre::Real	-0.999995231
[3]	Ogre::Real	-0.0144124255
[4]	Ogre::Real	0.0229277667
[5]	Ogre::Real	0.999732971
[6]	Ogre::Real	-0.000987402512
[7]	Ogre::Real	5.45578289
[8]	Ogre::Real	0.999733209
[9]	Ogre::Real	-0.0229268074
[10]	Ogre::Real	0.00101141329
[11]	Ogre::Real	-0.0305121373
[12]	Ogre::Real	-25439.875

this:
[0]	Ogre::Real	1
[1]	Ogre::Real	0
[2]	Ogre::Real	0
[3]	Ogre::Real	0
[4]	Ogre::Real	0
[5]	Ogre::Real	1
[6]	Ogre::Real	0
[7]	Ogre::Real	0
[8]	Ogre::Real	0
[9]	Ogre::Real	0
[10]	Ogre::Real	1
[11]	Ogre::Real	0
[12]	Ogre::Real	-0.00000984325925
[13]	Ogre::Real	0.570747256
[14]	Ogre::Real	0.56889689
[15]	Ogre::Real	0.0000144095939

Any idea why the incorrect data state here? Thanks much!