diff --git a/CHANGELOG-PRERELEASE.md b/CHANGELOG-PRERELEASE.md index c1e9d9896..b1125e88a 100644 --- a/CHANGELOG-PRERELEASE.md +++ b/CHANGELOG-PRERELEASE.md @@ -37,6 +37,7 @@ The format is based on [Keep a Changelog]. - Bones swung by unused PhysBones (which will be removed by AAO) are not merged `#850` - Note that To fix this problem, AnimatorParser is almost completely rewritten. - It's not expected to have behavior change, but if you found some, please report it. +- Re-fix Nested Constraint can be broken with Trace and Optimize `#880` ### Security diff --git a/CHANGELOG.md b/CHANGELOG.md index fc74180c1..fd5ac9e52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ The format is based on [Keep a Changelog]. - Bones swung by unused PhysBones (which will be removed by AAO) are not merged `#850` - Note that To fix this problem, AnimatorParser is almost completely rewritten. - It's not expected to have behavior change, but if you found some, please report it. +- Re-fix Nested Constraint can be broken with Trace and Optimize `#880` ### Security diff --git a/Editor/APIInternal/ComponentInfos.cs b/Editor/APIInternal/ComponentInfos.cs index ede9a0e31..cdf82d44a 100644 --- a/Editor/APIInternal/ComponentInfos.cs +++ b/Editor/APIInternal/ComponentInfos.cs @@ -330,15 +330,7 @@ protected override void CollectDependency(T component, ComponentDependencyCollec .EvenIfDependantDisabled(); for (var i = 0; i < component.sourceCount; i++) collector.AddDependency(component.GetSource(i).sourceTransform); - var isNestedConstraint = - component.GetComponentsInChildren() != null && - component.GetComponentsInParent() != null; - // for nested constraint, our optimizer may breaks the constraint - // https://github.com/anatawa12/AvatarOptimizer/issues/856 - if (isNestedConstraint) - collector.MarkBehaviour(); - else - collector.MarkHeavyBehaviour(); + collector.MarkHeavyBehaviour(); } } diff --git a/Editor/Processors/TraceAndOptimize/ComponentDependencyCollector.cs b/Editor/Processors/TraceAndOptimize/ComponentDependencyCollector.cs index abbf61a20..e97119e3e 100644 --- a/Editor/Processors/TraceAndOptimize/ComponentDependencyCollector.cs +++ b/Editor/Processors/TraceAndOptimize/ComponentDependencyCollector.cs @@ -100,13 +100,9 @@ public void Init(GCComponentInfo info) public MeshInfo2 GetMeshInfoFor(SkinnedMeshRenderer renderer) => _collector._session.GetMeshInfoFor(renderer); - public override void MarkEntrypoint() => _info.EntrypointComponent = true; - - public override void MarkHeavyBehaviour() => _info.HeavyBehaviourComponent = true; - public override void MarkBehaviour() - { - // currently NOP - } + public override void MarkEntrypoint() => _info.MarkEntrypoint(); + public override void MarkHeavyBehaviour() => _info.MarkHeavyBehaviour(); + public override void MarkBehaviour() => _info.MarkBehaviour(); private API.ComponentDependencyInfo AddDependencyInternal( [NotNull] GCComponentInfo info, diff --git a/Editor/Processors/TraceAndOptimize/FindUnusedObjectsProcessor.cs b/Editor/Processors/TraceAndOptimize/FindUnusedObjectsProcessor.cs index 14e956190..dffd4e673 100644 --- a/Editor/Processors/TraceAndOptimize/FindUnusedObjectsProcessor.cs +++ b/Editor/Processors/TraceAndOptimize/FindUnusedObjectsProcessor.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using nadena.dev.ndmf; @@ -21,14 +22,17 @@ protected override void Execute(BuildContext context, TraceAndOptimizeState stat internal readonly struct MarkObjectContext { private readonly GCComponentInfoHolder _componentInfos; + private readonly Func> _getDependantMap; private readonly Queue _processPending; private readonly Component _entrypoint; - public MarkObjectContext(GCComponentInfoHolder componentInfos, Component entrypoint) + public MarkObjectContext(GCComponentInfoHolder componentInfos, Component entrypoint, + Func> getDependantMap) { _componentInfos = componentInfos; _processPending = new Queue(); _entrypoint = entrypoint; + _getDependantMap = getDependantMap; } public void MarkComponent(Component component, @@ -37,14 +41,15 @@ public void MarkComponent(Component component, var dependencies = _componentInfos.TryGetInfo(component); if (dependencies == null) return; - if (dependencies.DependantEntrypoint.TryGetValue(_entrypoint, out var existingFlags)) + var dependantMap = _getDependantMap(dependencies); + if (dependantMap.TryGetValue(_entrypoint, out var existingFlags)) { - dependencies.DependantEntrypoint[_entrypoint] = existingFlags | type; + dependantMap[_entrypoint] = existingFlags | type; } else { _processPending.Enqueue(component); - dependencies.DependantEntrypoint.Add(_entrypoint, type); + dependantMap.Add(_entrypoint, type); } } @@ -94,6 +99,7 @@ public void ProcessNew() Sweep(componentInfos); if (!_noConfigureMergeBone) MergeBone(componentInfos); + MarkDependant(componentInfos); if (!_noActivenessAnimation) ActivenessAnimation(componentInfos); } @@ -113,7 +119,7 @@ private void ActivenessAnimation(GCComponentInfoHolder componentInfos) continue; // enabled is animated so we will not generate activeness animation HashSet resultSet; - using (var enumerator = componentInfo.DependantEntrypoint.Keys.GetEnumerator()) + using (var enumerator = componentInfo.DependantComponents.GetEnumerator()) { System.Diagnostics.Debug.Assert(enumerator.MoveNext()); resultSet = GetEntrypointActiveness(enumerator.Current, _context); @@ -227,7 +233,8 @@ private void Mark(GCComponentInfoHolder componentInfos) if (componentInfo.IsEntrypoint) { var component = componentInfo.Component; - var markContext = new MarkObjectContext(componentInfos, component); + + var markContext = new MarkObjectContext(componentInfos, component, GetDependantMap); markContext.MarkComponent(component, GCComponentInfo.DependencyType.Normal); markContext.MarkRecursively(); } @@ -235,7 +242,7 @@ private void Mark(GCComponentInfoHolder componentInfos) if (_exclusions.Count != 0) { // excluded GameObjects must be exists - var markContext = new MarkObjectContext(componentInfos, _context.AvatarRootTransform); + var markContext = new MarkObjectContext(componentInfos, _context.AvatarRootTransform, GetDependantMap); foreach (var gameObject in _exclusions) foreach (var component in gameObject.GetComponents()) @@ -244,6 +251,8 @@ private void Mark(GCComponentInfoHolder componentInfos) markContext.MarkRecursively(); } + Dictionary GetDependantMap(GCComponentInfo x) => + x.DependantEntrypoint; } private void Sweep(GCComponentInfoHolder componentInfos) @@ -268,6 +277,35 @@ private void Sweep(GCComponentInfoHolder componentInfos) } } + private void MarkDependant(GCComponentInfoHolder componentInfos) + { + // entrypoint for mark & sweep is active-able GameObjects + foreach (var componentInfo in componentInfos.AllInformation) + { + if (componentInfo.BehaviourComponent) + { + var component = componentInfo.Component; + var markContext = new MarkObjectContext(componentInfos, component, GetDependantMap); + markContext.MarkComponent(component, GCComponentInfo.DependencyType.Normal); + markContext.MarkRecursively(); + } + } + + if (_exclusions.Count != 0) { + // excluded GameObjects must be exists + var markContext = new MarkObjectContext(componentInfos, _context.AvatarRootTransform, GetDependantMap); + + foreach (var gameObject in _exclusions) + foreach (var component in gameObject.GetComponents()) + markContext.MarkComponent(component, GCComponentInfo.DependencyType.Normal); + + markContext.MarkRecursively(); + } + + Dictionary GetDependantMap(GCComponentInfo x) => + x.DependantBehaviours; + } + private void MergeBone(GCComponentInfoHolder componentInfos) { ConfigureRecursive(_context.AvatarRootTransform, _context); @@ -307,7 +345,7 @@ private void MergeBone(GCComponentInfoHolder componentInfos) // Components must be Transform Only if (transform.GetComponents().Length != 1) return NotMerged(); // The bone cannot be used generally - if ((componentInfos.GetInfo(transform).AllUsages & ~AllowedUsages) != 0) return NotMerged(); + if ((componentInfos.GetInfo(transform).AllEntrypointUsages & ~AllowedUsages) != 0) return NotMerged(); // must not be animated if (TransformAnimated(transform, context)) return NotMerged(); diff --git a/Editor/Processors/TraceAndOptimize/GCComponentInfo.cs b/Editor/Processors/TraceAndOptimize/GCComponentInfo.cs index 914ef0f61..d04f2ac2d 100644 --- a/Editor/Processors/TraceAndOptimize/GCComponentInfo.cs +++ b/Editor/Processors/TraceAndOptimize/GCComponentInfo.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using JetBrains.Annotations; using nadena.dev.ndmf; using UnityEngine; @@ -62,6 +63,11 @@ internal class GCComponentInfo /// public bool HeavyBehaviourComponent = false; + /// + /// True if activeness of this component has meaning + /// + public bool BehaviourComponent => _behaviourComponent || HeavyBehaviourComponent; + /// /// Dependencies of this component /// @@ -74,9 +80,18 @@ internal class GCComponentInfo [NotNull] internal readonly Dictionary DependantEntrypoint = new Dictionary(); + /// + /// Dependants entrypoint components + /// + [NotNull] internal readonly Dictionary DependantBehaviours = + new Dictionary(); + + internal IEnumerable DependantComponents => + DependantEntrypoint.Keys.Concat(DependantBehaviours.Keys); + internal readonly Component Component; - public DependencyType AllUsages + public DependencyType AllEntrypointUsages { get { @@ -90,6 +105,7 @@ public DependencyType AllUsages public bool IsEntrypoint => EntrypointComponent && Activeness != false; public readonly bool? Activeness; + private bool _behaviourComponent = false; public GCComponentInfo(Component component, bool? activeness) { @@ -98,6 +114,11 @@ public GCComponentInfo(Component component, bool? activeness) Activeness = activeness; } + + public void MarkEntrypoint() => EntrypointComponent = true; + public void MarkHeavyBehaviour() => HeavyBehaviourComponent = true; + public void MarkBehaviour() => _behaviourComponent = true; + [Flags] public enum DependencyType : byte {