Quick fix for skinning translation bug.

We weren't binding the transform of the skin to the base
so the transform was getting applied twice and also
the math was wrong in skinning.

This is the shortest change to make this work but it
is arguably not the *correct* change.

The correct change would be to make the import code
create Skin separate from ParamArray and SkinEval
so that you can use the same Skin data multiple
times in the same scene. I don't know enough about
collada to know if that's possible to setup in
collada but the way the code is structured currently
it seems to assume it is even though the code does
support this correctly.

That would take some work to fix and require samples
and it seems unlikely people will run into that issue
so this smaller fix seems good enough for now.

Review URL: http://codereview.chromium.org/235046

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27447 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 16 insertions, 10 deletions

diff --git a/o3d/core/cross/skin.cc b/o3d/core/cross/skin.cc
index c7fa8ff..3c438c6 100644
--- a/o3d/core/cross/skin.cc
+++ b/o3d/core/cross/skin.cc
@@ -482,7 +482,7 @@ void SkinEval::UpdateOutputs() {
           << " is not a ParamMatrix4";
       return;
     }
-    bones_[ii] = param->value() * inverse_base * inverse_bind_pose_array[ii];
+    bones_[ii] = inverse_base * param->value() * inverse_bind_pose_array[ii];
   }
 
   DoSkinning(the_skin);
diff --git a/o3d/import/cross/collada.cc b/o3d/import/cross/collada.cc
index e04c32a..2076921d 100644
--- a/o3d/import/cross/collada.cc
+++ b/o3d/import/cross/collada.cc
@@ -887,7 +887,8 @@ bool Collada::ImportTreeInstances(FCDocument* doc,
           if (controller->IsSkin()) {
             Shape* shape = GetSkinnedShape(doc,
                                            controller_instance,
-                                           node_instance);
+                                           node_instance,
+                                           transform);
             if (shape) {
               transform->AddShape(shape);
             } else {
@@ -1108,7 +1109,8 @@ Shape* Collada::GetShape(FCDocument* doc,
 
 Shape* Collada::GetSkinnedShape(FCDocument* doc,
                                 FCDControllerInstance* instance,
-                                NodeInstance *parent_node_instance) {
+                                NodeInstance *parent_node_instance,
+                                Transform* parent) {
   Shape* shape = NULL;
   FCDController* controller =
       static_cast<FCDController*>(instance->GetEntity());
@@ -1116,7 +1118,7 @@ Shape* Collada::GetSkinnedShape(FCDocument* doc,
     fm::string id = controller->GetDaeId();
     shape = skinned_shapes_[id.c_str()];
     if (!shape) {
-      shape = BuildSkinnedShape(doc, instance, parent_node_instance);
+      shape = BuildSkinnedShape(doc, instance, parent_node_instance, parent);
       if (!shape)
         return NULL;
       skinned_shapes_[id.c_str()] = shape;
@@ -1127,9 +1129,6 @@ Shape* Collada::GetSkinnedShape(FCDocument* doc,
 
 // Builds an O3D shape node corresponding to a given FCollada geometry
 // instance.
-// Parameters:
-//   doc:                The FCollada document from which to import nodes.
-//   geom_instance:      The FCollada geometry node to be converted.
 Shape* Collada::BuildShape(FCDocument* doc,
                            FCDGeometryInstance* geom_instance,
                            FCDGeometry* geom,
@@ -1312,8 +1311,11 @@ Shape* Collada::BuildShape(FCDocument* doc,
 
 Shape* Collada::BuildSkinnedShape(FCDocument* doc,
                                   FCDControllerInstance* instance,
-                                  NodeInstance *parent_node_instance) {
+                                  NodeInstance *parent_node_instance,
+                                  Transform* parent) {
   // TODO(o3d): Handle chained controllers. Morph->Skin->...
+  // TODO(gman): Change this to correctly create the skin, separate from
+  //     ParamArray and SkinEval so that we can support instanced skins.
   Shape* shape = NULL;
   LOG_ASSERT(doc && instance);
   FCDController* controller =
@@ -1392,6 +1394,8 @@ Shape* Collada::BuildSkinnedShape(FCDocument* doc,
 
     skin_eval->set_skin(skin);
     skin_eval->set_matrices(matrices);
+    skin_eval->GetParam<ParamMatrix4>(SkinEval::kBaseParamName)->Bind(
+        parent->GetParam<ParamMatrix4>(Transform::kWorldMatrixParamName));
 
     // Bind bones to matrices
     size_t num_bones = instance->GetJointCount();
diff --git a/o3d/import/cross/collada.h b/o3d/import/cross/collada.h
index c2f659f..1bac5ee 100644
--- a/o3d/import/cross/collada.h
+++ b/o3d/import/cross/collada.h
@@ -344,13 +344,15 @@ class Collada {
   // instance. If the Shape does not exist, Builds one.
   Shape* GetSkinnedShape(FCDocument* doc,
                          FCDControllerInstance* instance,
-                         NodeInstance *parent_node_instance);
+                         NodeInstance *parent_node_instance,
+                         Transform* parent);
 
   // Builds O3D skinned shape corresponding to a given FCollada controller
   // instance.
   Shape* BuildSkinnedShape(FCDocument* doc,
                            FCDControllerInstance* instance,
-                           NodeInstance *parent_node_instance);
+                           NodeInstance *parent_node_instance,
+                           Transform* parent);
 
   // Builds an O3D texture corresponding to a given FCollada surface
   // parameter.