From 00c683dd238d9b6942c1bb9c34e3c320678eddd1 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Sun, 9 Apr 2023 19:08:57 +0900 Subject: [PATCH] fix: retarget is not performed if rootBone is the only bone to be retargeted (#241) * fix: retarget is not performed if rootBone is the only bone to be retargeted * fix: MeshRetargeter may cause NRE if mesh is null * fix: MeshRetargeter is not working if sharedMesh is null * test: add test case for SkinnedMeshRenderer only with rootBone * test: fix RootBoneOnly test * test: fix expected and actual * test: add test for SkinnedMeshRenderer without mesh --- .../EditModeTests/RetargetMeshesTest.cs | 53 +++++++++++++++++++ .../EditModeTests/RetargetMeshesTest.cs.meta | 3 ++ .../Editor/MeshRetargeter.cs | 43 +++++++++------ 3 files changed, 83 insertions(+), 16 deletions(-) create mode 100644 Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs create mode 100644 Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs.meta diff --git a/Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs b/Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs new file mode 100644 index 00000000..d128df2f --- /dev/null +++ b/Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs @@ -0,0 +1,53 @@ +using nadena.dev.modular_avatar.core.editor; +using NUnit.Framework; +using UnityEngine; +using VRC.SDK3.Avatars.Components; + +namespace modular_avatar_tests +{ + public class RetargetMeshesTest : TestBase + { + // Real world case of this test case is with skinned mesh without bones or skinned mesh renderer with null mesh. + [Test] + public void RootBoneOnly() + { + var root = CreateRoot("root"); + var a = CreateChild(root, "a"); + var b = CreateChild(a, "b"); + + var skinnedMeshRenderer = root.AddComponent(); + skinnedMeshRenderer.sharedMesh = new Mesh(); + skinnedMeshRenderer.rootBone = b.transform; + Debug.Assert(skinnedMeshRenderer.bones.Length == 0); + + BoneDatabase.AddMergedBone(b.transform); + var context = new BuildContext(root.GetComponent()); + new RetargetMeshes().OnPreprocessAvatar(root, context); + + Assert.AreEqual(a.transform, skinnedMeshRenderer.rootBone); + } + + [Test] + public void NoMeshRootBoneOnly() + { + var root = CreateRoot("root"); + var a = CreateChild(root, "a"); + var b = CreateChild(a, "b"); + b.transform.localScale = new Vector3(2, 2, 2); + + var skinnedMeshRenderer = root.AddComponent(); + skinnedMeshRenderer.sharedMesh = null; + skinnedMeshRenderer.localBounds = new Bounds(new Vector3(0, 0, 0), new Vector3(1, 1, 1)); + skinnedMeshRenderer.rootBone = b.transform; + Debug.Assert(skinnedMeshRenderer.bones.Length == 0); + + BoneDatabase.AddMergedBone(b.transform); + var context = new BuildContext(root.GetComponent()); + new RetargetMeshes().OnPreprocessAvatar(root, context); + + Assert.AreEqual(a.transform, skinnedMeshRenderer.rootBone); + Assert.AreEqual(new Bounds(new Vector3(0, 0, 0), new Vector3(2, 2, 2)), + skinnedMeshRenderer.localBounds); + } + } +} diff --git a/Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs.meta b/Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs.meta new file mode 100644 index 00000000..3ad5d958 --- /dev/null +++ b/Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 0c1eeed79228433d8f9ee98d2ae852d1 +timeCreated: 1679738453 \ No newline at end of file diff --git a/Packages/nadena.dev.modular-avatar/Editor/MeshRetargeter.cs b/Packages/nadena.dev.modular-avatar/Editor/MeshRetargeter.cs index aadb1955..755fd746 100644 --- a/Packages/nadena.dev.modular-avatar/Editor/MeshRetargeter.cs +++ b/Packages/nadena.dev.modular-avatar/Editor/MeshRetargeter.cs @@ -24,6 +24,7 @@ using System.Collections.Generic; using System.Linq; +using JetBrains.Annotations; using nadena.dev.modular_avatar.editor.ErrorReporting; using UnityEditor; using UnityEngine; @@ -88,8 +89,6 @@ namespace nadena.dev.modular_avatar.core.editor { BuildReport.ReportingObject(renderer, () => { - if (renderer.sharedMesh == null) return; - bool isRetargetable = false; foreach (var bone in renderer.bones) { @@ -100,10 +99,12 @@ namespace nadena.dev.modular_avatar.core.editor } } + isRetargetable |= BoneDatabase.GetRetargetedBone(renderer.rootBone); + if (isRetargetable) { var newMesh = new MeshRetargeter(renderer).Retarget(); - _context.SaveAsset(newMesh); + if (newMesh) _context.SaveAsset(newMesh); } }); } @@ -143,13 +144,14 @@ namespace nadena.dev.modular_avatar.core.editor internal class MeshRetargeter { private readonly SkinnedMeshRenderer renderer; - private Mesh src, dst; + [CanBeNull] private Mesh src, dst; public MeshRetargeter(SkinnedMeshRenderer renderer) { this.renderer = renderer; } + [CanBeNull] public Mesh Retarget() { var avatar = RuntimeUtil.FindAvatarInParents(renderer.transform); @@ -165,8 +167,11 @@ namespace nadena.dev.modular_avatar.core.editor avatarTransform.localScale = Vector3.one; src = renderer.sharedMesh; - dst = Mesh.Instantiate(src); - dst.name = "RETARGETED: " + src.name; + if (src != null) + { + dst = Mesh.Instantiate(src); + dst.name = "RETARGETED: " + src.name; + } RetargetBones(); AdjustShapeKeys(); @@ -185,22 +190,25 @@ namespace nadena.dev.modular_avatar.core.editor private void RetargetBones() { - var originalBindPoses = src.bindposes; + var originalBindPoses = src ? src.bindposes : null; var originalBones = renderer.bones; var newBones = (Transform[]) originalBones.Clone(); - var newBindPoses = (Matrix4x4[]) originalBindPoses.Clone(); + var newBindPoses = (Matrix4x4[]) originalBindPoses?.Clone(); for (int i = 0; i < originalBones.Length; i++) { Transform newBindTarget = BoneDatabase.GetRetargetedBone(originalBones[i]); if (newBindTarget == null) continue; - - Matrix4x4 Bp = newBindTarget.worldToLocalMatrix * originalBones[i].localToWorldMatrix * - originalBindPoses[i]; - newBones[i] = newBindTarget; - newBindPoses[i] = Bp; + + if (originalBindPoses != null) + { + Matrix4x4 Bp = newBindTarget.worldToLocalMatrix * originalBones[i].localToWorldMatrix * + originalBindPoses[i]; + + newBindPoses[i] = Bp; + } } var rootBone = renderer.rootBone; @@ -212,9 +220,12 @@ namespace nadena.dev.modular_avatar.core.editor scaleBone = renderer.bones[0]; } - dst.bindposes = newBindPoses; renderer.bones = newBones; - renderer.sharedMesh = dst; + if (dst) + { + dst.bindposes = newBindPoses; + renderer.sharedMesh = dst; + } var newRootBone = BoneDatabase.GetRetargetedBone(rootBone, true); var newScaleBone = BoneDatabase.GetRetargetedBone(scaleBone, true); @@ -239,4 +250,4 @@ namespace nadena.dev.modular_avatar.core.editor renderer.probeAnchor = BoneDatabase.GetRetargetedBone(renderer.probeAnchor, true); } } -} \ No newline at end of file +}