Ticket #1012: collada_import_fixes-12092011.patch

source/collada/PMDConvert.cpp

@@ -252 +252 @@
                     // guaranteed by ReduceInfluences; necessary for avoiding
                     // out-of-bounds writes to the VertexBlend
 
+                if (vertexInfluences[i].GetPairCount() == 0)
+                {
+                    // Blender exports some models with vertices that have no influences,
+                    //  which I've not found details about in the COLLADA spec, however,
+                    //  it seems to work OK to treat these vertices the same as if they
+                    //  were only influenced by the bind-shape matrix (see comment below),
+                    //  so we use the same special case here.
+                    influences.bones[0] = (uint8)jointCount;
+                    influences.weights[0] = 1.0f;
+                }
+
                 for (size_t j = 0; j < vertexInfluences[i].GetPairCount(); ++j)
                 {
-                    uint32 jointIdx = vertexInfluences[i].GetPair(j)->jointIndex;
-                    REQUIRE(jointIdx <= 0xFF, "sensible number of joints (<256)"); // because we only have a u8 to store them in
+                    if (vertexInfluences[i].GetPair(j)->jointIndex == -1)
+                    {
+                        // This is a special case we must handle, according to the COLLADA spec:
+                        //  "An index of -1 into the array of joints refers to the bind shape"
+                        //  which basically means when skinning the vertex it's relative to the
+                        //  bind-shape transform instead of an animated bone. Since our skinning
+                        //  is in world space, we will have already applied the bind-shape transform,
+                        //  so we don't have to worry about that, though we DO have to apply the
+                        //  world space transform of the model for the indicated vertex.
+                        //
+                        //  To indicate this special case, we use a bone ID set to the total number
+                        //  of bones in the model, which will have a special "bone matrix" reserved
+                        //  that contains the world space transform of the model during skinning.
+                        //  (see http://trac.wildfiregames.com/ticket/1012)
+                        influences.bones[j] = (uint8)jointCount;
+                        influences.weights[j] = vertexInfluences[i].GetPair(j)->weight;
+                    }
+                    else
+                    {
+                        uint32 jointIdx = vertexInfluences[i].GetPair(j)->jointIndex;
+                        REQUIRE(jointIdx < 0xFF, "sensible number of joints (<255)"); // because we only have a u8 to store them in
 
-                    // Find the joint on the skeleton, after checking it really exists
-                    FCDSceneNode* joint = NULL;
-                    if (jointIdx < controllerInstance.GetJointCount())
-                        joint = controllerInstance.GetJoint(jointIdx);
+                        // Find the joint on the skeleton, after checking it really exists
+                        FCDSceneNode* joint = NULL;
+                        if (jointIdx < controllerInstance.GetJointCount())
+                            joint = controllerInstance.GetJoint(jointIdx);
 
-                    // Complain on error
-                    if (! joint)
-                    {
-                        if (! hasComplainedAboutNonexistentJoints)
+                        // Complain on error
+                        if (! joint)
                         {
-                            Log(LOG_WARNING, "Vertexes influenced by nonexistent joint");
-                            hasComplainedAboutNonexistentJoints = true;
+                            if (! hasComplainedAboutNonexistentJoints)
+                            {
+                                Log(LOG_WARNING, "Vertexes influenced by nonexistent joint");
+                                hasComplainedAboutNonexistentJoints = true;
+                            }
+                            continue;
                         }
-                        continue;
-                    }
 
-                    // Store into the VertexBlend
-                    int boneId = skeleton.GetBoneID(joint->GetName().c_str());
-                    if (boneId < 0)
-                    {
-                        // The relevant joint does exist, but it's not a recognised
-                        // bone in our chosen skeleton structure
-                        Log(LOG_ERROR, "Vertex influenced by unrecognised bone '%s'", joint->GetName().c_str());
-                        continue;
+                        // Store into the VertexBlend
+                        int boneId = skeleton.GetBoneID(joint->GetName().c_str());
+                        if (boneId < 0)
+                        {
+                            // The relevant joint does exist, but it's not a recognised
+                            // bone in our chosen skeleton structure
+                            Log(LOG_ERROR, "Vertex influenced by unrecognised bone '%s'", joint->GetName().c_str());
+                            continue;
+                        }
+
+                        influences.bones[j] = (uint8)boneId;
+                        influences.weights[j] = vertexInfluences[i].GetPair(j)->weight;
                     }
-
-                    influences.bones[j] = (uint8)boneId;
-                    influences.weights[j] = vertexInfluences[i].GetPair(j)->weight;
                 }
 
                 boneWeights.push_back(influences);
@@ -522 +553 @@
     /**
      * Applies world-space transform to vertex data and transforms Collada's right-handed
      *  Y-up / Z-up coordinates to the game's left-handed Y-up coordinate system
+     *
+     * TODO: Maybe we should use FCDocumentTools::StandardizeUpAxisAndLength in addition
+     *      to this, so we'd only have one up-axis case to worry about, but it doesn't seem to
+     *      correctly adjust the prop points in Y_UP models.
      */
     static void TransformStaticModel(float* position, float* normal, size_t vertexCount,
         const FMMatrix44& transform, bool yUp)
@@ -563 +598 @@
     /**
      * Applies world-space transform to vertex data and transforms Collada's right-handed
      *  Y-up / Z-up coordinates to the game's left-handed Y-up coordinate system
+     *
+     * TODO: Maybe we should use FCDocumentTools::StandardizeUpAxisAndLength in addition
+     *      to this, so we'd only have one up-axis case to worry about, but it doesn't seem to
+     *      correctly adjust the prop points in Y_UP models.
      */
     static void TransformSkinnedModel(float* position, float* normal, size_t vertexCount,
         std::vector<BoneTransform>& bones, std::vector<PropPoint>& propPoints,

source/graphics/Model.cpp

@@ -84 +84 @@
         size_t numBlends = modeldef->GetNumBlends();
 
         // allocate matrices for bone transformations
-        m_BoneMatrices = (CMatrix3D*)rtl_AllocateAligned(sizeof(CMatrix3D) * (numBones + numBlends), 16);
+        // (one extra matrix is used for the special case of bind-shape relative weighting)
+        m_BoneMatrices = (CMatrix3D*)rtl_AllocateAligned(sizeof(CMatrix3D) * (numBones + numBlends + 1), 16);
         for (size_t i = 0; i < numBones + numBlends; ++i)
         {
             m_BoneMatrices[i].SetIdentity();
@@ -398 +399 @@
             m_BoneMatrices[i] = m_BoneMatrices[i] * m_InverseBindBoneMatrices[i];
         }
 
+        // Note: there is a special case of joint influence, in which the vertex
+        //  is influenced by the bind-shape transform instead of a particular bone,
+        //  which we indicate with the blending bone ID set to the total number
+        //  of bones. But since we're skinning in world space, we use the model's
+        //  world space transform and store that matrix in this special index.
+        //  (see http://trac.wildfiregames.com/ticket/1012)
+        m_BoneMatrices[m_pModelDef->GetNumBones()] = m_Transform;
+
         m_pModelDef->BlendBoneMatrices(m_BoneMatrices);
     }
 }

source/graphics/ModelDef.cpp

@@ -209 +209 @@
     for (size_t i = 0; i < m_NumBlends; ++i)
     {
         const SVertexBlend& blend = m_pBlends[i];
-        CMatrix3D& boneMatrix = boneMatrices[m_NumBones + i];
+        CMatrix3D& boneMatrix = boneMatrices[m_NumBones + 1 + i];
+        
+        // Note: there is a special case of joint influence, in which the vertex
+        //  is influenced by the bind-shape matrix instead of a particular bone,
+        //  which we indicate by setting the bone ID set to the total number
+        //  of bones. It should be blended with the world space transform and we
+        //  have already set up this matrix in boneMatrices.
+        //  (see http://trac.wildfiregames.com/ticket/1012)
+
         boneMatrix.Blend(boneMatrices[blend.m_Bone[0]], blend.m_Weight[0]);
         boneMatrix.AddBlend(boneMatrices[blend.m_Bone[1]], blend.m_Weight[1]);
         for (size_t j = 2; j < SVertexBlend::SIZE && blend.m_Bone[j] != 0xFF; ++j)
@@ -301 +309 @@
                 }
                 if (j >= blends.size())
                     blends.push_back(blend);
-                mdef->m_pBlendIndices[i] = mdef->m_NumBones + j;
+                // This index is offset by one to allow the special case of a
+                //  weighted influence relative to the bind-shape rather than
+                //  a particular bone. See comment in BlendBoneMatrices.
+                mdef->m_pBlendIndices[i] = mdef->m_NumBones + j + 1;
             }
         }