UE 5.7 Import SkeletalMesh with MorphTargets and UseT0AsRefPose enabled Crash

Development Character & Animation
, ,
1

Our animators suddenly started crashing when trying to import SkeletalMeshes with MorphTargets after we upgraded to 5.7.3

Import to note: We’re using the legacy importer, not interchange.

Just from testing it looks like it only happens with UseT0AsRefPose set to true when importing.

And the crash was an array out of bounds exception in FbxSkeletalMeshImport.cpp FFbxImporter::SkinControlPointsToPose line 491 when accessing the ImportData.Faces.

It’s far out of my domain of expertise, but following the stack trace it seems that they call a FSkeletalMeshImportData::CopyDataNeedByMorphTargetImport to copy only a subset of the import data to work with when working on importing the morph targets.

And that function does not copy the faces, which have recently become important to have as of this Changelist https://github.com/EpicGames/UnrealEngine/commit/cc00d739ca05bf80487523e7ef73dd39a21e83f5#diff-dda815166b9e40cbd55ed36f621dfbdc44e59789907ce25982160ec4619f391a

I wrote what seems like a sensible fix

void FSkeletalMeshImportData::CopyDataNeedByMorphTargetImport(FSkeletalMeshImportData& Other) const
{
    //The points array is the only data we need to compute the morph target in the skeletalmesh build
    Other.Points = Points;
    //PointToRawMap should not be save when saving morph target data, we only need it temporary to gather the point from the fbx shape
    Other.PointToRawMap = PointToRawMap;
 
    //New Code!!!
    // ImportMorphTargetsInternal was updated to use Faces, Normals, Tangents, and binormalSigns if UseT0AsRefPose is toggled on
    // https://github.com/EpicGames/UnrealEngine/commit/cc00d739ca05bf80487523e7ef73dd39a21e83f5#diff-dda815166b9e40cbd55ed36f621dfbdc44e59789907ce25982160ec4619f391a
    // But ImportData didn't have the faces it assumed to have, leading to a crash.
    Other.Faces = Faces;    
    Other.bHasNormals = bHasNormals;
    Other.bHasTangents = bHasTangents;
    // !!
}

Attached an Image of what settings we have enabled when importing fbx skeletal mesh, and the stack trace of the crash

[Attachment Removed]

2

Steps to Reproduce

  1. Grab a SkeletalMesh (With Morph Targets) made in Maya (that’s just what we use)
  2. Import into Unreal
  3. Make sure UseT0AsRefPose and Import Morph Targets is enabled
  4. See that it crashes

[Attachment Removed]

3

Hi, thanks for taking the time to report this.

It looks like this codepath was just missed when the CL that you mentioned was implemented. The change you included looks good to me, but I’ve asked the interchange team to validate that it doesn’t have any unintended consequences elsewhere. Any code that uses the Faces array to index into the Wedges array would still fail with this change, but considering that hasn’t been a problem previously, I think we’re fine not to also copy that data. I’ll follow up once I’ve heard from the devs.

[Attachment Removed]

4

Thanks for looking into it. Good to hear that it makes sense.

I’ll keep an ear open for any follow up.

[Attachment Removed]

5

The dev team were happy with those changes, so I’ve gone ahead and committed them. They’ll be part of the 5.8 release.

Also, I’m just curious if there’s a particular reason that you’re using legacy importer?

[Attachment Removed]

6

Thanks!

Word from our animation team (I wasn’t here when the decision was originally made):

With legacy importer they can import animations they made that affect Blendshapes and Bones into one animation sequence.

With the new importer, at least at the time that they tried it, it would import those animations as two sequences: one for blendshapes, and another for bones.

And they couldn’t find a way to have the old behaviour they preferred without going back to the legacy importer, which they didn’t have a problem with.

Again, not an area I’ve spent too much time in, so if you think that doesn’t make sense, and there is in fact a way to import as a single anim sequence I’m all ears! I’d be happy to help them out.

[Attachment Removed]

7

Ok yeah, that makes sense. I remember that issue; it was actually fixed in one of the 5.5 patch releases. The CL with the fix is 38538158 if you wanted to take a look. So if you wanted to switch back to using Interchange as the import pipeline, it should work for you. We would recommend that generally, as the legacy pipeline isn’t being maintained particularly well at this point (as you’ve seen with the current issue). If you were to test out Interchange again and found any problems, just let us know, and we can look at fixing them.

[Attachment Removed]

8

Oh that’s great!

I’ll communicate it with the team.

Thanks for your help with all of this

[Attachment Removed]

9

No problem, I’ll close out this thread

[Attachment Removed]