Merge pull request #214 from anatawa12/fix-error-merge-skinned-mesh

Fix error merge skinned mesh
anatawa12 · Jun 6, 2023 · 5c8715c · 5c8715c
2 parents 75b04e3 + 5f6339c
commit 5c8715c
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 16 deletions.
diff --git a/CHANGELOG-PRERELEASE.md b/CHANGELOG-PRERELEASE.md
@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog].
 ### Removed
 
 ### Fixed
+- Error in MergeSkinnedMeshProcessor with RecordMoveProperty `#214`
 
 ### Security
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog].
 ### Removed
 
 ### Fixed
+- Error in MergeSkinnedMeshProcessor with RecordMoveProperty `#214`
 
 ### Security
 

diff --git a/Editor/ObjectMapping/ObjectMappingBuilder.cs b/Editor/ObjectMapping/ObjectMappingBuilder.cs
@@ -48,8 +48,11 @@ public ObjectMappingBuilder([NotNull] GameObject rootObject)
         public void RecordMergeComponent<T>(T from, T mergeTo) where T: Component =>
             GetComponentInfo(from).MergedTo(GetComponentInfo(mergeTo));
 
+        public void RecordMoveProperties(Component from, params (string old, string @new)[] props) =>
+            GetComponentInfo(from).MoveProperties(props);
+
         public void RecordMoveProperty(Component from, string oldProp, string newProp) =>
-            GetComponentInfo(from).MoveProperty(oldProp, newProp);
+            GetComponentInfo(from).MoveProperties((oldProp, newProp));
 
         public void RecordRemoveProperty(Component from, string oldProp) =>
             GetComponentInfo(from).RemoveProperty(oldProp);
@@ -96,26 +99,45 @@ public void MergedTo([NotNull] BuildingComponentInfo mergeTo)
                 _mergedInto = mergeTo;
             }
 
-            public void MoveProperty(string oldProp, string newProp)
+            public void MoveProperties(params (string old, string @new)[] props)
             {
-                foreach (var mergeSource in MergeSources) mergeSource.MoveProperty(oldProp, newProp);
+                foreach (var mergeSource in MergeSources) mergeSource.MoveProperties(props);
+
+                var propertyIds = new int[props.Length];
+                for (var i = 0; i < props.Length; i++)
+                {
+                    var (oldProp, newProp) = props[i];
+                    if (_afterPropertyIds.TryGetValue(oldProp, out var propId))
+                    {
+                        propertyIds[i] = propId;
+                    }
+                    else
+                    {
+                        if (!_beforePropertyIds.ContainsKey(oldProp))
+                        {
+                            if (_afterPropertyIds.ContainsKey(newProp) && props.All(x => x.old != newProp))
+                                throw new InvalidOperationException("Merging property");
+                            propertyIds[i] = _nextPropertyId++;
+                        }
+                    }
+                }
 
-                if (_afterPropertyIds.TryGetValue(oldProp, out var propId))
+                for (var i = 0; i < propertyIds.Length; i++)
                 {
+                    var propId = propertyIds[i];
+                    var (oldProp, _) = props[i];
+                    if (propId == 0) continue;
                     _afterPropertyIds.Remove(oldProp);
-                    _afterPropertyIds[newProp] = propId;
-                    if (!_beforePropertyIds.ContainsKey(oldProp))
-                        _beforePropertyIds.Add(oldProp, propId);
                 }
-                else
+
+                for (var i = 0; i < propertyIds.Length; i++)
                 {
+                    var propId = propertyIds[i];
+                    var (oldProp, newProp) = props[i];
+                    if (propId == 0) continue;
+                    _afterPropertyIds[newProp] = propId;
                     if (!_beforePropertyIds.ContainsKey(oldProp))
-                    {
-                        if (_afterPropertyIds.ContainsKey(newProp))
-                            throw new InvalidOperationException("Merging property");
-                        _beforePropertyIds.Add(oldProp, propId = _nextPropertyId++);
-                        _afterPropertyIds.Add(newProp, propId);
-                    }
+                        _beforePropertyIds.Add(oldProp, propId);
                 }
             }
 

diff --git a/Editor/Processors/SkinnedMeshes/MergeSkinnedMeshProcessor.cs b/Editor/Processors/SkinnedMeshes/MergeSkinnedMeshProcessor.cs
@@ -33,6 +33,8 @@ public override void Process(OptimizerSession session, MeshInfo2 target, MeshInf
             TexCoordStatus TexCoordStatusMax(TexCoordStatus x, TexCoordStatus y) =>
                 (TexCoordStatus)Math.Max((int)x, (int)y);
 
+            var mappings = new List<(string, string)>();
+
             for (var i = 0; i < meshInfos.Length; i++)
             {
                 var meshInfo = meshInfos[i];
@@ -47,6 +49,8 @@ TexCoordStatus TexCoordStatusMax(TexCoordStatus x, TexCoordStatus y) =>
                 for (var j = 0; j < meshInfo.SubMeshes.Count; j++)
                     target.SubMeshes[subMeshIndexMap[i][j]].Triangles.AddRange(meshInfo.SubMeshes[j].Triangles);
 
+                mappings.Clear();
+
                 // add blend shape if not defined by name
                 for (var sourceI = 0; sourceI < meshInfo.BlendShapes.Count; sourceI++)
                 {
@@ -58,10 +62,12 @@ TexCoordStatus TexCoordStatusMax(TexCoordStatus x, TexCoordStatus y) =>
                         target.BlendShapes.Add((name, weight));
                     }
 
-                    session.MappingBuilder.RecordMoveProperty(meshInfo.SourceRenderer, 
-                        VProp.BlendShapeIndex(sourceI), VProp.BlendShapeIndex(newIndex));
+                    // this can cause merge prop error.
+                    mappings.Add((VProp.BlendShapeIndex(sourceI), VProp.BlendShapeIndex(newIndex)));
                 }
 
+                session.MappingBuilder.RecordMoveProperties(meshInfo.SourceRenderer, mappings.ToArray());
+
                 target.Bones.AddRange(meshInfo.Bones);
 
                 target.HasColor |= meshInfo.HasColor;

diff --git a/Test~/ObjectMappingTest.cs b/Test~/ObjectMappingTest.cs
@@ -156,6 +156,32 @@ public void RecordMovePropertyTest()
                 Is.EqualTo(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.changed")));
         }
 
+        [Test]
+        public void RecordSwapPropertyTest()
+        {
+            var root = new GameObject();
+            var child1 = Utils.NewGameObject("child1", root.transform);
+            var child1Component = child1.AddComponent<SkinnedMeshRenderer>();
+
+            var builder = new ObjectMappingBuilder(root);
+            builder.RecordMoveProperties(child1Component, 
+                ("blendShapes.first", "blendShapes.second"),
+                ("blendShapes.second", "blendShapes.first"));
+
+            var built = builder.BuildObjectMapping();
+
+            var rootMapper = built.CreateAnimationMapper(root);
+
+            // but should affect to component
+            Assert.That(
+                rootMapper.MapBinding(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.first")),
+                Is.EqualTo(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.second")));
+
+            Assert.That(
+                rootMapper.MapBinding(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.second")),
+                Is.EqualTo(B("child1", typeof(SkinnedMeshRenderer), "blendShapes.first")));
+        }
+
         [Test]
         public void RecordMovePropertyTwiceTest()
         {