Fixed issues with blend shape animations (#3080)

Duplicate blend shape animations are now handled correctly.
Invalid animation targets are now an error instead of a crash in the builder.

Signed-off-by: stankowi <[email protected]>

Code/Tools/SceneAPI/SDKWrapper/AssImpSceneWrapper.cpp

@@ -70,6 +70,9 @@ namespace AZ
             // This results in the loss of the offset matrix data for nodes without a mesh which is required for the Transform Importer.
             m_importer.SetPropertyBool(AI_CONFIG_IMPORT_FBX_PRESERVE_PIVOTS, false);
             m_importer.SetPropertyBool(AI_CONFIG_IMPORT_FBX_OPTIMIZE_EMPTY_ANIMATION_CURVES, false);
+            // The remove empty bones flag is on by default, but doesn't do anything internal to AssImp right now.
+            // This is here as a bread crumb to save others times investigating issues with empty bones.
+            // m_importer.SetPropertyBool(AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES, false);
             m_sceneFileName = fileName;
             m_assImpScene = m_importer.ReadFile(fileName,
                 aiProcess_Triangulate //Triangulates all faces of all meshes

Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpAnimationImporter.cpp

@@ -147,7 +147,9 @@ namespace AZ
                 SerializeContext* serializeContext = azrtti_cast<SerializeContext*>(context);
                 if (serializeContext)
                 {
-                    serializeContext->Class<AssImpAnimationImporter, SceneCore::LoadingComponent>()->Version(5); // [LYN-4226] Invert PostRotation matrix in animation chains
+                    // Revision 5: [LYN-4226] Invert PostRotation matrix in animation chains
+                    // Revision 6: Handle duplicate blend shape animations
+                    serializeContext->Class<AssImpAnimationImporter, SceneCore::LoadingComponent>()->Version(6);
                 }
             }
 
@@ -631,6 +633,15 @@ namespace AZ
 
                 for (const auto& [meshIdx, keys] : valueToKeyDataMap)
                 {
+
+                    if (static_cast<AZ::u32>(meshIdx) >= mesh->mNumAnimMeshes)
+                    {
+                        AZ_Error(
+                            "AnimationImporter", false,
+                            "Mesh %s has an animation mesh index reference of %d, but only has %d animation meshes. Skipping importing this. This is an error in the source scene file that should be corrected.",
+                            mesh->mName.C_Str(), meshIdx, mesh->mNumAnimMeshes);
+                        continue;
+                    }
                     AZStd::shared_ptr<SceneData::GraphData::BlendShapeAnimationData> morphAnimNode =
                         AZStd::make_shared<SceneData::GraphData::BlendShapeAnimationData>();
 
@@ -656,12 +667,29 @@ namespace AZ
                         morphAnimNode->AddKeyFrame(weight);
                     }
 
+                    // Some DCC tools, like Maya, include a full path separated by '.' in the node names.
+                    // For example, "cone_skin_blendShapeNode.cone_squash"
+                    // Downstream processing doesn't want anything but the last part of that node name,
+                    // so find the last '.' and remove anything before it.
                     const size_t dotIndex = nodeName.find_last_of('.');
                     nodeName = nodeName.substr(dotIndex + 1);
 
                     morphAnimNode->SetBlendShapeName(nodeName.data());
 
-                    AZStd::string animNodeName(AZStd::string::format("%s_%s", s_animationNodeName, nodeName.data()));
+                    // Duplicates can exist if an anim mesh had a name with a suffix like .001, in that case
+                    // AssImp will strip off that suffix. Note that this behavior is separate from the
+                    // scan for a period in the node name that came before this.
+                    AZStd::string originalNodeName(AZStd::string::format("%s_%s", s_animationNodeName, nodeName.data()));
+                    AZStd::string animNodeName(originalNodeName);
+                    if (RenamedNodesMap::SanitizeNodeName(
+                        animNodeName, context.m_scene.GetGraph(), context.m_currentGraphPosition, originalNodeName.c_str()))
+                    {
+                        AZ_Warning(
+                            "AnimationImporter", false,
+                            "Duplicate animations were found with the name %s on mesh %s. The duplicate will be named %s.",
+                            originalNodeName.c_str(), mesh->mName.C_Str(), animNodeName.c_str());
+                    }
+
                     Containers::SceneGraph::NodeIndex addNode = context.m_scene.GetGraph().AddChild(
                         context.m_currentGraphPosition, animNodeName.c_str(), AZStd::move(morphAnimNode));
                     context.m_scene.GetGraph().MakeEndPoint(addNode);

Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBlendShapeImporter.cpp

@@ -40,7 +40,9 @@ namespace AZ
                 SerializeContext* serializeContext = azrtti_cast<SerializeContext*>(context);
                 if (serializeContext)
                 {
-                    serializeContext->Class<AssImpBlendShapeImporter, SceneCore::LoadingComponent>()->Version(3); // LYN-2576
+                    // Revision 3: Fixed an issue where jack.fbx was failing to process
+                    // Revision 4: Handle duplicate blend shape animations
+                    serializeContext->Class<AssImpBlendShapeImporter, SceneCore::LoadingComponent>()->Version(4);
                 }
             }
 
@@ -80,7 +82,32 @@ namespace AZ
                 // AssImp separates meshes that have multiple materials.
                 // This code re-combines them to match previous FBX SDK behavior,
                 // so they can be separated by engine code instead.
-                AZStd::map<AZStd::string_view, AZStd::vector<AZStd::pair<int, int>>> animToMeshToAnimMeshIndices;
+                // Can't de-dupe nodes in the first loop because we can't generate names until we create nodes later.
+                // Because meshes are split on material at this point and need to be recombined, we can be in a position where
+                // There is a legit duped anim mesh that needs to be combined based on the outer non-anim mesh,
+                // or this is a duplicately named anim mesh that needs to be de-duped. There is also the case where both are true,
+                // it's a duplicate name and the non-anim mesh has to be deduped.
+
+                // Helper struct to track an anim mesh and its associated mesh.
+                struct AnimMeshAndSceneMeshIndex
+                {
+                    AnimMeshAndSceneMeshIndex(const aiAnimMesh* aiAnimMesh, const aiMesh* aiMesh)
+                        : m_aiAnimMesh(aiAnimMesh)
+                        , m_aiMesh(aiMesh)
+                    {
+                    }
+                    const aiAnimMesh* m_aiAnimMesh = nullptr;
+                    const aiMesh* m_aiMesh = nullptr;
+                };
+
+                // Helper struct to track all anim meshes at an index for all scene meshes.
+                struct AnimMeshAndSceneMeshes
+                {
+                    AZStd::vector<AnimMeshAndSceneMeshIndex> m_animMeshAndSceneMeshIndex;
+                };
+
+                // Map the animation index to the list of anim meshes at that index, and the mesh associated with those anim meshes.
+                AZStd::map<int, AnimMeshAndSceneMeshes> animMeshIndexToSceneMeshes;
                 for (int nodeMeshIdx = 0; nodeMeshIdx < numMesh; nodeMeshIdx++)
                 {
                     int sceneMeshIdx = context.m_sourceNode.GetAssImpNode()->mMeshes[nodeMeshIdx];
@@ -88,20 +115,57 @@ namespace AZ
                     for (unsigned int animIdx = 0; animIdx < aiMesh->mNumAnimMeshes; animIdx++)
                     {
                         aiAnimMesh* aiAnimMesh = aiMesh->mAnimMeshes[animIdx];
-                        animToMeshToAnimMeshIndices[aiAnimMesh->mName.C_Str()].emplace_back(nodeMeshIdx, animIdx);
+
+                        // This code executes if:
+                        //  A mesh in the FBX file had multiple materials and blend shapes.
+                        //  This means that AssImp splits that mesh to one material per mesh.
+                        //  AssImp creates a set of anim meshes for each mesh based on that split.
+                        // This verifies that those anim mesh arrays are in the same order across all split meshes, if it fails
+                        // it means this logic needs to be updated, but it also catches that here earlier in an obvious way,
+                        // instead of failing later in a harder to track way.
+                        if (animMeshIndexToSceneMeshes.contains(animIdx))
+                        {
+                            const AnimMeshAndSceneMeshIndex& firstExistingAnim(
+                                animMeshIndexToSceneMeshes[animIdx].m_animMeshAndSceneMeshIndex[0]);
+                            if (strcmp(
+                                    firstExistingAnim.m_aiAnimMesh->mName.C_Str(),
+                                    aiAnimMesh->mName.C_Str()) != 0)
+                            {
+                                AZ_Error(
+                                    Utilities::ErrorWindow, false,
+                                    "Meshes %s and %s on node %s have mismatched animations %s and %s at index %d. This can be resolved by "
+                                    "either manually separating meshes by material in the source scene file, or by updating this logic to "
+                                    "handle out of order animation indices.",
+                                    firstExistingAnim.m_aiMesh->mName.C_Str(),
+                                    aiMesh->mName.C_Str(),
+                                    context.m_sourceNode.GetName(),
+                                    firstExistingAnim.m_aiAnimMesh->mName.C_Str(),
+                                    aiAnimMesh->mName.C_Str(), animIdx);
+                                return Events::ProcessingResult::Failure;
+                            }
+                        }
+
+                        animMeshIndexToSceneMeshes[animIdx].m_animMeshAndSceneMeshIndex.emplace_back(
+                            AnimMeshAndSceneMeshIndex(aiAnimMesh, aiMesh));
                     }
                 }
 
-                for (const auto& animToMeshIndex : animToMeshToAnimMeshIndices)
+                for (const auto& animMeshToSceneMeshes : animMeshIndexToSceneMeshes)
                 {
                     AZStd::shared_ptr<SceneData::GraphData::BlendShapeData> blendShapeData =
                         AZStd::make_shared<SceneData::GraphData::BlendShapeData>();
 
+                    if (animMeshToSceneMeshes.second.m_animMeshAndSceneMeshIndex.size() == 0)
+                    {
+                        AZ_Error(Utilities::ErrorWindow, false, "Blend shape animations were expected but missing on node %s.",
+                            context.m_sourceNode.GetName());
+                        return Events::ProcessingResult::Failure;
+                    }
                     // Some DCC tools, like Maya, include a full path separated by '.' in the node names.
                     // For example, "cone_skin_blendShapeNode.cone_squash"
                     // Downstream processing doesn't want anything but the last part of that node name,
                     // so find the last '.' and remove anything before it.
-                    AZStd::string nodeName(animToMeshIndex.first);
+                    AZStd::string nodeName(animMeshToSceneMeshes.second.m_animMeshAndSceneMeshIndex[0].m_aiAnimMesh->mName.C_Str());
                     size_t dotIndex = nodeName.rfind('.');
                     if (dotIndex != AZStd::string::npos)
                     {
@@ -109,12 +173,11 @@ namespace AZ
                     }
                     int vertexOffset = 0;
                     RenamedNodesMap::SanitizeNodeName(nodeName, context.m_scene.GetGraph(), context.m_currentGraphPosition, "BlendShape");
-                    AZ_TraceContext("Blend shape name", nodeName);
-                    for (const auto& meshIndex : animToMeshIndex.second)
+
+                    for (const auto& animMeshAndSceneIndex : animMeshToSceneMeshes.second.m_animMeshAndSceneMeshIndex)
                     {
-                        int sceneMeshIdx = context.m_sourceNode.GetAssImpNode()->mMeshes[meshIndex.first];
-                        const aiMesh* aiMesh = context.m_sourceScene.GetAssImpScene()->mMeshes[sceneMeshIdx];
-                        const aiAnimMesh* aiAnimMesh = aiMesh->mAnimMeshes[meshIndex.second];
+                        const aiAnimMesh* aiAnimMesh = animMeshAndSceneIndex.m_aiAnimMesh;
+                        const aiMesh* aiMesh = animMeshAndSceneIndex.m_aiMesh;
 
                         AZStd::bitset<SceneData::GraphData::BlendShapeData::MaxNumUVSets> uvSetUsedFlags;
                         for (AZ::u8 uvSetIndex = 0; uvSetIndex < SceneData::GraphData::BlendShapeData::MaxNumUVSets; ++uvSetIndex)
@@ -199,6 +262,7 @@ namespace AZ
                                     face.mNumIndices);
                                 continue;
                             }
+
                             for (unsigned int idx = 0; idx < face.mNumIndices; ++idx)
                             {
                                 blendFace.vertexIndex[idx] = face.mIndices[idx] + vertexOffset;
@@ -207,11 +271,8 @@ namespace AZ
                             blendShapeData->AddFace(blendFace);
                         }
                         vertexOffset += aiMesh->mNumVertices;
-
-
                     }
 
-
                     // Report problem if no vertex or face converted to MeshData
                     if (blendShapeData->GetVertexCount() <= 0 || blendShapeData->GetFaceCount() <= 0)
                     {