Array out of bounds in UE::Anim::FAttributeBlendData::GetBoneWeight due to missing remapping of bone index into compact pose

We have an animation blueprint that uses control rig to set an animation attribute. We were getting fatal asserts when executing this in certain situations. The issue was that the blend operation was looking up oob bones based on an index in the animation attributes.

On investigation we noticed something odd, duplicate entries in the data set:

`- BlendAttributes Num=1, Max=4 const TArray<UE::Anim::FStackAttributeContainer,TSizedDefaultAllocator<32>> &

  • [0] {…} UE::Anim::FStackAttributeContainer
  • UE::Anim::TAttributeContainer<FCompactPoseBoneIndex,TMemStackAllocator<0> > {UniqueTypedBoneIndices=Num=1, Max=1 AttributeIdentifiers=Num=1, Max=1 Values=Num=1, Max=1 …} UE::Anim::TAttributeContainer<FCompactPoseBoneIndex,TMemStackAllocator<0>>
  • UniqueTypedBoneIndices Num=1, Max=1 TArray<TArray<int,TSizedDefaultAllocator<32>>,TSizedDefaultAllocator<32>>
  • AttributeIdentifiers Num=1, Max=1 TArray<TArray<UE::Anim::FAttributeId,TSizedDefaultAllocator<32>>,TSizedDefaultAllocator<32>>
  • [0] Num=7, Max=7 TArray<UE::Anim::FAttributeId,TSizedDefaultAllocator<32>>
  • [0] {Namespace=“bone” Name=“camera_ref” Index=149 } UE::Anim::FAttributeId
  • [1] {Namespace=“bone” Name=“RootMotionDelta” Index=0 } UE::Anim::FAttributeId
  • [2] {Namespace=“bone” Name=“CameraSpace” Index=64 } UE::Anim::FAttributeId
  • [3] {Namespace=“bone” Name=“CameraSpace” Index=18 } UE::Anim::FAttributeId
  • [4] {Namespace=“bone” Name=“CameraSpace” Index=6 } UE::Anim::FAttributeId
  • [5] {Namespace=“bone” Name=“camera_ref” Index=155 } UE::Anim::FAttributeId
  • [6] {Namespace=“bone” Name=“camera_original” Index=155 } UE::Anim::FAttributeId
  • [Raw View] {AllocatorInstance={…} ArrayNum=7 ArrayMax=7 } TArray<UE::Anim::FAttributeId,TSizedDefaultAllocator<32>>
  • [Raw View] {AllocatorInstance={…} ArrayNum=1 ArrayMax=1 } TArray<TArray<UE::Anim::FAttributeId,TSizedDefaultAllocator<32>>,TSizedDefaultAllocator<32>>
  • Values Num=1, Max=1 TArray<TArray<UE::Anim::TWrappedAttribute<TMemStackAllocator<0>>,TSizedDefaultAllocator<32>>,TSizedDefaultAllocator<32>>
  • UniqueTypes Num=1, Max=2 TArray<TWeakObjectPtr<UScriptStruct,FWeakObjectPtr>,TSizedDefaultAllocator<32>>The code for setting the attributes from the ABP lives in \FRigDispatch_SetAnimAttribute`. This calls `GetAnimAttributeValue`.

The code in here calls `OwningComponent->GetSkeletalMeshAsset()->GetRefSkeleton().FindBoneIndex(BoneName)` which gets a bone index in skeleton space. However, it does directly c-style cast it to `FCompactPoseBoneIndex` to create the animation attribute. Going by other code, it instead should be remapped first using `GetCompactPoseIndexFromSkeletonIndex` or similar from `FBoneContainer`.

Quick proof of concept fix resolves the issue, but it is messy (const cast and everything) since getting to the bone container is problematic at first glance in this location. Someone with more logic of the animation code/control rig should come up with something that is more suitable:

// Invalidate cache if input changed if (CachedBoneName != BoneName) { const int32 SkeletonBoneIndex = OwningComponent->GetSkeletalMeshAsset()->GetRefSkeleton().FindBoneIndex(BoneName); if (SkeletonBoneIndex != INDEX_NONE) { const FCompactPoseBoneIndex CompactPoseBoneIndex = const_cast<USkeletalMeshComponent*>(OwningComponent)->GetSharedRequiredBones()->GetCompactPoseIndexFromSkeletonIndex(SkeletonBoneIndex); if (CompactPoseBoneIndex != INDEX_NONE) { CachedBoneIndex = int32(CompactPoseBoneIndex); } } }

Hi, thanks for reporting this. This issue has come up previously, but the way that you’re grabbing the required bones from the mesh in order to solve it is a good option for a fix. However, your fix only resolves one of the potential bugs here which is when the bone name specified on the SetAnimAttribute rig unit doesn’t map directly from skeleton index to compact pose index. But there’s another related bug hiding here which is what happens when the bone doesn’t exist at all after a switch of LOD.

I took a look at how to solve that one as well, but you essentially have to disregard the mechanism that we have in the code to cache the bone name and index, since we can’t rely on those being correct when the LOD changes. But we also have no way to know (currently) that the LOD has changed within the rig unit. So you have to do the work to calculate the bone index every frame like this:

if (BoneName == NAME_None) { // default to use root bone CachedBoneIndex = 0; } else { // Get the compact pose bone index to ensure we have the correct index for the current LOD const TSharedPtr<struct FBoneContainer> RequiredBones = OwningComponent->GetSharedRequiredBones(); CachedBoneIndex = OwningComponent->GetSkeletalMeshAsset()->GetRefSkeleton().FindBoneIndex(BoneName); const FCompactPoseBoneIndex PoseBoneIndex = RequiredBones->GetCompactPoseIndexFromSkeletonPoseIndex(FSkeletonPoseBoneIndex(CachedBoneIndex)); CachedBoneIndex = PoseBoneIndex.GetInt(); }With that change, both the bug that you found and the other with the bone not existing after an LOD switch should be resolved. I don’t want to submit this change, though, since if we’re going to do it this way, there’s no point in caching the bone name or index. So we could just remove that. Another option might be to get the current LOD value from the RequiredBones, cache that as well and check it at the same point where we are currently checking whether the bone name has changed.

Note that you’ll probably also want to make this change in the non-templated implementation of GetAnimAttributeValue which exists in RigUnit_AnimAttribute.cpp. And that you don’t need to do the const cast if you change the proceeding code in both functions that gets the owning skeletal mesh component from using GetOwningComponent to GetMutableOwningComponent. ie. from this:

const USkeletalMeshComponent* OwningComponent = Cast<USkeletalMeshComponent>(Context.GetOwningComponent());to this:

USkeletalMeshComponent* OwningComponent = Cast<USkeletalMeshComponent>(Context.GetMutableOwningComponent());

Thank you, since you said you don’t want to submit this changes, are you going to be tracking this internally and thus there will be a proper fix further down the line in UE? If so, mind sharing the `UE-` ticket number so we can keep an eye out for it?

Sure, it’ll be fixed as part of UE-218079. I talked with the dev team about this today and we’re going to go with a different approach as part of that JIRA. Currently, all the control rig code is using FStackAttributeContainer, which requires compact bone indices. But we also have FMeshAttributeContainer which uses mesh bones instead. If we switch the control rig code over to using this attribute container type, all the issues that we’ve been discussing on this thread go away, since the existing code is getting the bone index from the mesh, and the bones will always exist in the mesh (even if they aren’t skinned at a certain LOD).

We’re aiming to get this into 5.6, so if you keep an eye on the JIRA, there should be an update soon.