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

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>();
+            skinnedMeshRenderer.sharedMesh = new Mesh();
+            skinnedMeshRenderer.rootBone = b.transform;
+            Debug.Assert(skinnedMeshRenderer.bones.Length == 0);
+
+            BoneDatabase.AddMergedBone(b.transform);
+            var context = new BuildContext(root.GetComponent<VRCAvatarDescriptor>());
+            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>();
+            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<VRCAvatarDescriptor>());
+            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);
+        }
+    }
+}

Assets/_ModularAvatar/EditModeTests/RetargetMeshesTest.cs.meta

@@ -0,0 +1,3 @@
+fileFormatVersion: 2
+guid: 0c1eeed79228433d8f9ee98d2ae852d1
+timeCreated: 1679738453

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;
+            if (src != null)
+            {
                 dst = Mesh.Instantiate(src);
                 dst.name = "RETARGETED: " + src.name;
+            }
 
             RetargetBones();
             AdjustShapeKeys();
@@ -185,23 +190,26 @@ 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;
+                newBones[i] = newBindTarget;
 
+                if (originalBindPoses != null)
+                {
                     Matrix4x4 Bp = newBindTarget.worldToLocalMatrix * originalBones[i].localToWorldMatrix *
                                    originalBindPoses[i];
 
-                newBones[i] = newBindTarget;
                     newBindPoses[i] = Bp;
                 }
+            }
 
             var rootBone = renderer.rootBone;
             var scaleBone = rootBone;
@@ -212,9 +220,12 @@ namespace nadena.dev.modular_avatar.core.editor
                 scaleBone = renderer.bones[0];
             }
 
-            dst.bindposes = newBindPoses;
             renderer.bones = newBones;
+            if (dst)
+            {
+                dst.bindposes = newBindPoses;
                 renderer.sharedMesh = dst;
+            }
 
             var newRootBone = BoneDatabase.GetRetargetedBone(rootBone, true);
             var newScaleBone = BoneDatabase.GetRetargetedBone(scaleBone, true);