pafuhana1213 / KawaiiPhysics

KawaiiPhysics : Simple fake Physics for UnrealEngine4 & 5
MIT License
1.92k stars 287 forks source link

Crash when switching to LOD with LOD bone removal settings. #48

Closed okqi closed 1 year ago

okqi commented 2 years ago

FAnimNode_KawaiiPhysics::InitModifyBones

//CalcBoneLength(ModifyBones[0], BoneContainer.GetRefPoseCompactArray()); CalcBoneLength(ModifyBones[0], BoneContainer.GetRefPoseArray());

pafuhana1213 commented 2 years ago

Hi, I tried it but this issue did not occur. Would you please share with me how to repro it?

thanks

sergi-gonzalez commented 2 years ago

Hi @pafuhana1213 ,

First of all, our team loves Kawaii Physics, thanks a lot for sharing your work.

I am testing the plugin on UE5 and I got the same crash in the same place: CalcBoneLength(ModifyBones[0], Bones); \Plugins\KawaiiPhysics\Source\KawaiiPhysics\Private\AnimNode_KawaiiPhysics.cpp LN 200

I have investigated the problem and the crash happens because Bone.BoneRef.BoneIndex is greater than the number of transforms in RefBonePose. In our case LOD0 has 140 bones and LOD2 has 78 bones. The crash happened when InitModifyBones is called when BoneContainer.CalculatedForLOD is 2 and one modify bone had the boneIndex 90 (RefBonePose contains 78 entries).

The following fix seems to work as expected (changes tagged with [hexworks]):

void FAnimNode_KawaiiPhysics::CalcBoneLength(FKawaiiPhysicsModifyBone& Bone, const TArray<FTransform>& RefBonePose, const FBoneContainer& BoneContainer)//[hexworks] - Added BoneContainer
{
    if (Bone.ParentIndex < 0)
    {
        Bone.LengthFromRoot = 0.0f;
    }
    else
    {
        if (!Bone.bDummy)
        {
            FCompactPoseBoneIndex CompactPoseBoneIndex = Bone.BoneRef.GetCompactPoseIndex(BoneContainer);//[hexworks] - Fix crash indexing Bone.BoneRef.BoneIndex
            Bone.LengthFromRoot = ModifyBones[Bone.ParentIndex].LengthFromRoot
                + RefBonePose[CompactPoseBoneIndex.GetInt()].GetLocation().Size();//[hexworks] - Fix crash indexing Bone.BoneRef.BoneIndex
        }
        else
        {
            Bone.LengthFromRoot = ModifyBones[Bone.ParentIndex].LengthFromRoot + DummyBoneLength;
        }

        TotalBoneLength = FMath::Max(TotalBoneLength, Bone.LengthFromRoot);
    }

    for (int ChildIndex : Bone.ChildIndexs)
    {
        CalcBoneLength(ModifyBones[ChildIndex], RefBonePose, BoneContainer);//[hexworks] - Added BoneContainer
    }
}

It's also required to fix the lines that call CalcBoneLength adding BoneContainer as last parameter. Could you please check the fix and integrate it in the repo?

Best regards,

sergi

taka1arg commented 1 year ago

お疲れ様です。

私の方でも配列のオーバーランが発生し CalcBoneLength(ModifyBones[0], BoneContainer.GetRefPoseArray()); に変更して対応しました。

レベル起動後にスケルタルメッシュをカメラの遠く(LODが適用される距離)にスポーンすると発生ように思われます。

pafuhana1213 commented 1 year ago

Hi guys, Sorry for my late reply... I succeeded to repro this issue on UE5.1 and committed https://github.com/pafuhana1213/KawaiiPhysics/commit/a9bf985168eeacbacb651ccf54943bb0697ae88c to fix this now.

thanks!