Possible Bug in AnimSequencerDataModel when Evaluating Attributes on a Different Skeleton from Source

Development Character & Animation
, ,
1

Hello!

I was hunting down some bugs with regard to extracting attributes onto a Skeleton that’s different from the one that the Anim Data Model was created for. It looks like this was accounted for in UAnimSequence in CL 46151991, but there’s a hidden EvaluateAttributes inside AnimSequencerDataModel that may have the same bug.

for (const FAnimatedBoneAttribute& Attribute : AnimatedBoneAttributes)
{
    const FCompactPoseBoneIndex PoseBoneIndex = RequiredBones.GetCompactPoseIndexFromSkeletonPoseIndex(FSkeletonPoseBoneIndex(Attribute.Identifier.GetBoneIndex()));
    // Only add attribute if the bone its tied to exists in the currently evaluated set of bones
    if(PoseBoneIndex.IsValid())
    {
       UE::Anim::Attributes::GetAttributeValue(InOutPoseData.GetAttributes(), PoseBoneIndex, Attribute, EvaluationContext.SampleFrameRate.AsSeconds(EvaluationContext.SampleTime));
    }
}

This likely needs the same or similar remapping code that’s in UAnimSequence if the Skeleton is different. I haven’t tested this bug myself since we use a different AnimDataModel for loading perf reasons, but I suspect this bug exists. (There’s also an interesting side note which is why these attributes are extracted here and in UAnimSequence, but that’s another topic).

Anyway, I hope this is helpful!

-Nathaniel

[Attachment Removed]

2

Hi Nathaniel,

Thanks for the heads up on this. You’re right that it looks like this code was missed when those other changes were implemented. Catching all of the places in the codebase that should be using the skeleton remapping hasn’t been trivial. After a quick search, I think that we’re also missing the same code in UAnimDataModel::Evaluate and ExtractPose (in AnimDataModel.cpp). I’ll get these fixed up.

Thanks,

Euan

[Attachment Removed]

3

Yeah, you’re right about the UAnimDataModel implementations. In the end, I didn’t change those since we don’t really have content to test against that codepath now. But I’ve submitted the change in AnimSequencerDataModel so that’ll go into the 5.8 release. Thanks again for the heads up on this.

[Attachment Removed]

4

No problem! Indeed, I actually changed that one already, but didn’t expect y’all to change it since UAnimDataModel seemed relatively deprecated (though it’s way faster). Thanks Euan!

[Attachment Removed]