Fixed some issues with cloning of nodes (specific to RenderState), where a crash could occur if you tried to release the original node. The new node also shared some of the state of the original, where it should have been making a copy.

Fixes issues with rendering scenes that nest models within joint hierarchies.

A side effect of the above fix is that joints are now always visited when calling Scene::visit() and it is also possible to have the same joint visited more than once for complex nodes that have multiple models referencing the same joint.

Fixed an issue where cloning a node that had certain material auto bindings could crash if the node was not part of a scene before cloning it.

gameplay/src/Joint.cpp

@@ -6,7 +6,7 @@ namespace gameplay
 {
 
 Joint::Joint(const char* id)
-    : Node(id), _jointMatrixDirty(true), _skinCount(0)
+    : Node(id), _jointMatrixDirty(true)
 {
 }
 
@@ -25,7 +25,6 @@ Node* Joint::cloneSingleNode(NodeCloneContext &context) const
     GP_ASSERT(copy);
     context.registerClonedNode(this, copy);
     copy->_bindPose = _bindPose;
-    copy->_skinCount = _skinCount;
     Node::cloneInto(copy, context);
     return copy;
 }
@@ -35,6 +34,27 @@ Node::Type Joint::getType() const
     return Node::JOINT;
 }
 
+Scene* Joint::getScene() const
+{
+    // Overrides Node::getScene() to search the node our skins.
+    for (const SkinReference* itr = &_skin; itr && itr->skin; itr = itr->next)
+    {
+        Model* model = itr->skin ? itr->skin->getModel() : NULL;
+        if (model)
+        {
+            Node* node = model->getNode();
+            if (node)
+            {
+                Scene* scene = node->getScene();
+                if (scene)
+                    return scene;
+            }
+        }
+    }
+
+    return Node::getScene();
+}
+
 void Joint::transformChanged()
 {
     Node::transformChanged();
@@ -47,7 +67,7 @@ void Joint::updateJointMatrix(const Matrix& bindShape, Vector4* matrixPalette)
     // the _jointMatrixDirty optimization since updateJointMatrix() may be
     // called multiple times a frame with different bindShape matrices (and
     // different matrixPallete pointers).
-    if (_skinCount > 1 || _jointMatrixDirty)
+    if (_skin.next || _jointMatrixDirty)
     {
         _jointMatrixDirty = false;
 
@@ -73,4 +93,70 @@ void Joint::setInverseBindPose(const Matrix& m)
     _jointMatrixDirty = true;
 }
 
+void Joint::addSkin(MeshSkin* skin)
+{
+    if (!_skin.skin)
+    {
+        // Store skin in root reference
+        _skin.skin = skin;
+    }
+    else
+    {
+        // Add a new SkinReference to the end of our list
+        SkinReference* ref = &_skin;
+        while (ref->next)
+        {
+            ref = ref->next;
+        }
+        ref->next = new SkinReference();
+        ref->next->skin = skin;
+    }
+}
+
+void Joint::removeSkin(MeshSkin* skin)
+{
+    if (_skin.skin == skin)
+    {
+        // Skin is our root referenced skin
+        _skin.skin = NULL;
+
+        // Shift the next skin reference down to the root
+        if (_skin.next)
+        {
+            SkinReference* tmp = _skin.next;
+            _skin.skin = tmp->skin;
+            _skin.next = tmp->next;
+            tmp->next = NULL; // prevent deletion
+            SAFE_DELETE(tmp);
+        }
+    }
+    else
+    {
+        // Search for the entry referencing this skin
+        SkinReference* ref = &_skin;
+        while (SkinReference* tmp = ref->next)
+        {
+            if (tmp->skin == skin)
+            {
+                // Link this refernce out
+                ref->next = tmp->next;
+                tmp->next = NULL; // prevent deletion
+                SAFE_DELETE(tmp);
+                break;
+            }
+            ref = tmp;
+        }
+    }
+}
+
+Joint::SkinReference::SkinReference()
+    : skin(NULL), next(NULL)
+{
+}
+
+Joint::SkinReference::~SkinReference()
+{
+    SAFE_DELETE(next);
+}
+
 }

gameplay/src/Joint.h

@@ -25,6 +25,11 @@ public:
      */
     Node::Type getType() const;
 
+    /**
+     * @see Node::getScene()
+     */
+    Scene* getScene() const;
+
     /**
      * Returns the inverse bind pose matrix for this joint.
      * 
@@ -85,6 +90,18 @@ protected:
 
 private:
 
+    /**
+     * Internal structure to track mesh skins referencing a joint.
+     */
+    struct SkinReference
+    {
+        MeshSkin* skin;
+        SkinReference* next;
+
+        SkinReference();
+        ~SkinReference();
+    };
+
     /**
      * Constructor.
      */
@@ -95,22 +112,24 @@ private:
      */
     Joint& operator=(const Joint&);
 
-protected:
+    void addSkin(MeshSkin* skin);
+
+    void removeSkin(MeshSkin* skin);
 
     /** 
      * The Matrix representation of the Joint's bind pose.
      */
     Matrix _bindPose;
-    
-    /** 
+
+    /**
      * Flag used to mark if the Joint's matrix is dirty.
      */
     bool _jointMatrixDirty;
-    
-    /** 
-     * The number of MeshSkin's influencing the Joint.
+
+    /**
+     * Linked list of mesh skins that are referenced by this joint.
      */
-    unsigned int _skinCount;
+    SkinReference _skin;
 };
 
 }

gameplay/src/MeshSkin.cpp

@@ -141,7 +141,7 @@ void MeshSkin::setJoint(Joint* joint, unsigned int index)
 
     if (_joints[index])
     {
-        _joints[index]->_skinCount--;
+        _joints[index]->removeSkin(this);
         SAFE_RELEASE(_joints[index]);
     }
 
@@ -150,7 +150,7 @@ void MeshSkin::setJoint(Joint* joint, unsigned int index)
     if (joint)
     {
         joint->addRef();
-        joint->_skinCount++;
+        joint->addSkin(this);
     }
 }
 

gameplay/src/MeshSkin.h

@@ -21,6 +21,7 @@ class MeshSkin : public Transform::Listener
     friend class Model;
     friend class Joint;
     friend class Node;
+    friend class Scene;
 
 public:
 

gameplay/src/Model.h

@@ -20,6 +20,7 @@ class NodeCloneContext;
 class Model : public Ref
 {
     friend class Node;
+    friend class Scene;
     friend class Mesh;
     friend class Bundle;
 

gameplay/src/Node.cpp

@@ -376,13 +376,15 @@ unsigned int Node::findNodes(const char* id, std::vector<Node*>& nodes, bool rec
 
 Scene* Node::getScene() const
 {
-    // Search for a scene in our parents.
-    for (Node* n = const_cast<Node*>(this); n != NULL; n = n->getParent())
+    if (_scene)
+        return _scene;
+
+    // Search our parent for the scene
+    if (_parent)
     {
-        if (n->_scene)
-        {
-            return n->_scene;
-        }
+        Scene* scene = _parent->getScene();
+        if (scene)
+            return scene;
     }
 
     return NULL;
@@ -933,7 +935,6 @@ const BoundingSphere& Node::getBoundingSphere() const
     return _bounds;
 }
 
-
 Node* Node::clone() const
 {
     NodeCloneContext context;

gameplay/src/Node.h

@@ -223,7 +223,7 @@ public:
      *
      * @return The scene.
      */
-    Scene* getScene() const;
+    virtual Scene* getScene() const;
 
     /**
      * Gets the top level node in this node's parent hierarchy.

gameplay/src/RenderState.cpp

@@ -208,6 +208,8 @@ void RenderState::setNodeBinding(Node* node)
 
 void RenderState::applyAutoBinding(const char* uniformName, const char* autoBinding)
 {
+    GP_ASSERT(_nodeBinding);
+
     MaterialParameter* param = getParameter(uniformName);
     GP_ASSERT(param);
 
@@ -264,31 +266,19 @@ void RenderState::applyAutoBinding(const char* uniformName, const char* autoBind
     }
     else if (strcmp(autoBinding, "MATRIX_PALETTE") == 0)
     {
-        Model* model = _nodeBinding->getModel();
-        MeshSkin* skin = model ? model->getSkin() : NULL;
-        if (skin)
-        {
-            GP_ASSERT(param);
-            param->bindValue(skin, &MeshSkin::getMatrixPalette, &MeshSkin::getMatrixPaletteSize);
-        }
+        param->bindValue(this, &RenderState::autoBindingGetMatrixPalette, &RenderState::autoBindingGetMatrixPaletteSize);
     }
     else if (strcmp(autoBinding, "SCENE_AMBIENT_COLOR") == 0)
     {
-        Scene* scene = _nodeBinding->getScene();
-        if (scene)
-            param->bindValue(scene, &Scene::getAmbientColor);
+        param->bindValue(this, &RenderState::autoBindingGetAmbientColor);
     }
     else if (strcmp(autoBinding, "SCENE_LIGHT_COLOR") == 0)
     {
-        Scene* scene = _nodeBinding->getScene();
-        if (scene)
-            param->bindValue(scene, &Scene::getLightColor);
+        param->bindValue(this, &RenderState::autoBindingGetLightColor);
     }
     else if (strcmp(autoBinding, "SCENE_LIGHT_DIRECTION") == 0)
     {
-        Scene* scene = _nodeBinding->getScene();
-        if (scene)
-            param->bindValue(scene, &Scene::getLightDirection);
+        param->bindValue(this, &RenderState::autoBindingGetLightDirection);
     }
     else
     {
@@ -296,6 +286,39 @@ void RenderState::applyAutoBinding(const char* uniformName, const char* autoBind
     }
 }
 
+const Vector4* RenderState::autoBindingGetMatrixPalette() const
+{
+    Model* model = _nodeBinding ? _nodeBinding->getModel() : NULL;
+    MeshSkin* skin = model ? model->getSkin() : NULL;
+    return skin ? skin->getMatrixPalette() : NULL;
+}
+
+const Vector3& RenderState::autoBindingGetAmbientColor() const
+{
+    Scene* scene = _nodeBinding ? _nodeBinding->getScene() : NULL;
+    return scene ? scene->getAmbientColor() : Vector3::zero();
+}
+
+const Vector3& RenderState::autoBindingGetLightColor() const
+{
+    Scene* scene = _nodeBinding ? _nodeBinding->getScene() : NULL;
+    return scene ? scene->getLightColor() : Vector3::one();
+}
+
+const Vector3& RenderState::autoBindingGetLightDirection() const
+{
+    static Vector3 down(0, -1, 0);
+    Scene* scene = _nodeBinding ? _nodeBinding->getScene() : NULL;
+    return scene ? scene->getLightDirection() : down;
+}
+
+unsigned int RenderState::autoBindingGetMatrixPaletteSize() const
+{
+    Model* model = _nodeBinding ? _nodeBinding->getModel() : NULL;
+    MeshSkin* skin = model ? model->getSkin() : NULL;
+    return skin ? skin->getMatrixPaletteSize() : 0;
+}
+
 void RenderState::bind(Pass* pass)
 {
     GP_ASSERT(pass);
@@ -359,6 +382,7 @@ void RenderState::cloneInto(RenderState* renderState, NodeCloneContext& context)
 {
     GP_ASSERT(renderState);
 
+    // Clone parameters
     for (std::map<std::string, std::string>::const_iterator it = _autoBindings.begin(); it != _autoBindings.end(); ++it)
     {
         renderState->setParameterAutoBinding(it->first.c_str(), it->second.c_str());
@@ -373,13 +397,18 @@ void RenderState::cloneInto(RenderState* renderState, NodeCloneContext& context)
 
         renderState->_parameters.push_back(paramCopy);
     }
-    renderState->_parent = _parent;
+
+    // Clone our state block
     if (_state)
     {
-        renderState->setStateBlock(_state);
+        // StateBlock contains only simple primitive data, so use the default assignment
+        // operator to do a memberwise copy.
+        *renderState->getStateBlock() = *_state;
     }
-
-    // Note that _nodeBinding is not set here, it should be set by the caller.
+    
+    // Notes:
+    // 1. _nodeBinding should not be set here, it should be set by the caller.
+    // 2. _parent should not be set here, since it's set in the constructor of Technique and Pass.
 }
 
 RenderState::StateBlock::StateBlock()

gameplay/src/RenderState.h

@@ -2,6 +2,8 @@
 #define RENDERSTATE_H_
 
 #include "Ref.h"
+#include "Vector3.h"
+#include "Vector4.h"
 
 namespace gameplay
 {
@@ -461,6 +463,31 @@ private:
      */
     RenderState& operator=(const RenderState&);
 
+    /**
+     * Internal auto binding handler.
+     */
+    const Vector3& autoBindingGetAmbientColor() const;
+
+    /**
+     * Internal auto binding handler.
+     */
+    const Vector3& autoBindingGetLightColor() const;
+
+    /**
+     * Internal auto binding handler.
+     */
+    const Vector3& autoBindingGetLightDirection() const;
+
+    /**
+     * Internal auto binding handler.
+     */
+    const Vector4* autoBindingGetMatrixPalette() const;
+
+    /**
+     * Internal auto binding handler.
+     */
+    unsigned int autoBindingGetMatrixPaletteSize() const;
+
 protected:
 
     /**

gameplay/src/Scene.cpp

@@ -136,6 +136,30 @@ unsigned int Scene::findNodes(const char* id, std::vector<Node*>& nodes, bool re
     return count;
 }
 
+void Scene::visitNode(Node* node, const char* visitMethod)
+{
+    ScriptController* sc = Game::getInstance()->getScriptController();
+
+    // Invoke the visit method for this node.
+    if (!sc->executeFunction<bool>(visitMethod, "<Node>", node))
+        return;
+
+    // If this node has a model with a mesh skin, visit the joint hierarchy within it
+    // since we don't add joint hierarcies directly to the scene. If joints are never
+    // visited, it's possible that nodes embedded within the joint hierarchy that contain
+    // models will never get visited (and therefore never get drawn).
+    if (node->_model && node->_model->_skin && node->_model->_skin->_rootNode)
+    {
+        visitNode(node->_model->_skin->_rootNode, visitMethod);
+    }
+
+    // Recurse for all children.
+    for (Node* child = node->getFirstChild(); child != NULL; child = child->getNextSibling())
+    {
+        visitNode(child, visitMethod);
+    }
+}
+
 Node* Scene::addNode(const char* id)
 {
     Node* node = Node::create(id);

gameplay/src/Scene.h

@@ -333,7 +333,7 @@ private:
     /**
      * Visits the given node and all of its children recursively.
      */
-    inline void visitNode(Node* node, const char* visitMethod);
+    void visitNode(Node* node, const char* visitMethod);
 
     std::string _id;
     Camera* _activeCamera;
@@ -356,7 +356,6 @@ void Scene::visit(T* instance, bool (T::*visitMethod)(Node*))
     }
 }
 
-
 template <class T, class C>
 void Scene::visit(T* instance, bool (T::*visitMethod)(Node*,C), C cookie)
 {
@@ -381,6 +380,15 @@ void Scene::visitNode(Node* node, T* instance, bool (T::*visitMethod)(Node*))
     if (!(instance->*visitMethod)(node))
         return;
 
+    // If this node has a model with a mesh skin, visit the joint hierarchy within it
+    // since we don't add joint hierarcies directly to the scene. If joints are never
+    // visited, it's possible that nodes embedded within the joint hierarchy that contain
+    // models will never get visited (and therefore never get drawn).
+    if (node->_model && node->_model->_skin && node->_model->_skin->_rootNode)
+    {
+        visitNode(node->_model->_skin->_rootNode, instance, visitMethod);
+    }
+
     // Recurse for all children.
     for (Node* child = node->getFirstChild(); child != NULL; child = child->getNextSibling())
     {
@@ -395,25 +403,19 @@ void Scene::visitNode(Node* node, T* instance, bool (T::*visitMethod)(Node*,C),
     if (!(instance->*visitMethod)(node, cookie))
         return;
 
-    // Recurse for all children.
-    for (Node* child = node->getFirstChild(); child != NULL; child = child->getNextSibling())
+    // If this node has a model with a mesh skin, visit the joint hierarchy within it
+    // since we don't add joint hierarcies directly to the scene. If joints are never
+    // visited, it's possible that nodes embedded within the joint hierarchy that contain
+    // models will never get visited (and therefore never get drawn).
+    if (node->_model && node->_model->_skin && node->_model->_skin->_rootNode)
     {
-        visitNode(child, instance, visitMethod, cookie);
+        visitNode(node->_model->_skin->_rootNode, instance, visitMethod, cookie);
     }
-}
-
-inline void Scene::visitNode(Node* node, const char* visitMethod)
-{
-    ScriptController* sc = Game::getInstance()->getScriptController();
-
-    // Invoke the visit method for this node.
-    if (!sc->executeFunction<bool>(visitMethod, "<Node>", node))
-        return;
 
     // Recurse for all children.
     for (Node* child = node->getFirstChild(); child != NULL; child = child->getNextSibling())
     {
-        visitNode(child, visitMethod);
+        visitNode(child, instance, visitMethod, cookie);
     }
 }