From 7a2385352ca61417b390299dc435a270b876f8f2 Mon Sep 17 00:00:00 2001 From: bd_ Date: Mon, 26 Feb 2024 01:03:16 -0800 Subject: [PATCH] fix: noisy memory leak warning on Unity 2019 (#700) Closes: #696 --- Runtime/ArmatureAwase/ArmatureLock.cs | 41 +++++++++ ...atureLock.cs.meta => ArmatureLock.cs.meta} | 0 .../ArmatureAwase/ArmatureLockController.cs | 49 +++++----- .../BidirectionalArmatureLock.cs | 18 ++-- Runtime/ArmatureAwase/DeferDestroy.cs | 22 +++++ Runtime/ArmatureAwase/IArmatureLock.cs | 11 --- Runtime/ArmatureAwase/OnewayArmatureLock.cs | 26 +++--- .../BlendshapeSyncIntegrationTest.prefab | 89 ++++++++----------- 8 files changed, 148 insertions(+), 108 deletions(-) create mode 100644 Runtime/ArmatureAwase/ArmatureLock.cs rename Runtime/ArmatureAwase/{IArmatureLock.cs.meta => ArmatureLock.cs.meta} (100%) delete mode 100644 Runtime/ArmatureAwase/IArmatureLock.cs diff --git a/Runtime/ArmatureAwase/ArmatureLock.cs b/Runtime/ArmatureAwase/ArmatureLock.cs new file mode 100644 index 00000000..daed42cb --- /dev/null +++ b/Runtime/ArmatureAwase/ArmatureLock.cs @@ -0,0 +1,41 @@ +using System; + +namespace nadena.dev.modular_avatar.core.armature_lock +{ + internal abstract class ArmatureLock : IDisposable + { + private bool _enableAssemblyReloadCallback; + + protected bool EnableAssemblyReloadCallback + { + get => _enableAssemblyReloadCallback; + set + { + if (_enableAssemblyReloadCallback == value) return; + _enableAssemblyReloadCallback = value; +#if UNITY_EDITOR + if (value) + { + UnityEditor.AssemblyReloadEvents.beforeAssemblyReload += OnDomainUnload; + } + else + { + UnityEditor.AssemblyReloadEvents.beforeAssemblyReload -= OnDomainUnload; + } +#endif + } + } + + public abstract void Prepare(); + public abstract LockResult Execute(); + public abstract bool IsStable(); + public abstract void Dispose(); + + private void OnDomainUnload() + { + // Unity 2019 does not call deferred callbacks before domain unload completes, + // so we need to make sure to immediately destroy all our TransformAccessArrays. + DeferDestroy.DestroyImmediate(this); + } + } +} \ No newline at end of file diff --git a/Runtime/ArmatureAwase/IArmatureLock.cs.meta b/Runtime/ArmatureAwase/ArmatureLock.cs.meta similarity index 100% rename from Runtime/ArmatureAwase/IArmatureLock.cs.meta rename to Runtime/ArmatureAwase/ArmatureLock.cs.meta diff --git a/Runtime/ArmatureAwase/ArmatureLockController.cs b/Runtime/ArmatureAwase/ArmatureLockController.cs index d5ac4667..877bb5c3 100644 --- a/Runtime/ArmatureAwase/ArmatureLockController.cs +++ b/Runtime/ArmatureAwase/ArmatureLockController.cs @@ -18,21 +18,20 @@ namespace nadena.dev.modular_avatar.core.armature_lock internal static ArmatureLockConfig instance { get; } = new ArmatureLockConfig(); #endif - [SerializeField] - private bool _globalEnable = true; - + [SerializeField] private bool _globalEnable = true; + internal bool GlobalEnable { get => _globalEnable; set { if (value == _globalEnable) return; - - #if UNITY_EDITOR + +#if UNITY_EDITOR Undo.RecordObject(this, "Toggle Edit Mode Bone Sync"); Menu.SetChecked(UnityMenuItems.TopMenu_EditModeBoneSync, value); - #endif - +#endif + _globalEnable = value; if (!value) @@ -47,11 +46,12 @@ namespace nadena.dev.modular_avatar.core.armature_lock [InitializeOnLoadMethod] static void Init() { - EditorApplication.delayCall += () => { + EditorApplication.delayCall += () => + { Menu.SetChecked(UnityMenuItems.TopMenu_EditModeBoneSync, instance._globalEnable); }; } - + [MenuItem(UnityMenuItems.TopMenu_EditModeBoneSync, false, UnityMenuItems.TopMenu_EditModeBoneSyncOrder)] static void ToggleBoneSync() { @@ -74,7 +74,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock private readonly ModularAvatarMergeArmature _mama; private readonly GetTransformsDelegate _getTransforms; - private IArmatureLock _lock; + private ArmatureLock _lock; private bool GlobalEnable => ArmatureLockConfig.instance.GlobalEnable; private bool _updateActive; @@ -134,7 +134,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock public ArmatureLockController(ModularAvatarMergeArmature mama, GetTransformsDelegate getTransforms) { #if UNITY_EDITOR - AssemblyReloadEvents.beforeAssemblyReload += Dispose; + AssemblyReloadEvents.beforeAssemblyReload += OnDomainUnload; #endif this._mama = mama; @@ -165,7 +165,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock { UpdateLoopPrepare(); } - + private void UpdateLoopFinish() { DoFinish(); @@ -178,7 +178,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock } private bool IsPrepared = false; - + private void UpdateLoopPrepare() { if (_mama == null || !_mama.gameObject.scene.IsValid()) @@ -186,7 +186,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock UpdateActive = false; return; } - + if (!Enabled) { UpdateActive = false; @@ -201,7 +201,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock _lock = null; return; } - + if (_curMode == _mode) { _lock?.Prepare(); @@ -212,7 +212,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock private bool DoFinish() { LockResult result; - + if (!GlobalEnable) { _lock?.Dispose(); @@ -222,9 +222,9 @@ namespace nadena.dev.modular_avatar.core.armature_lock var wasPrepared = IsPrepared; IsPrepared = false; - + if (!Enabled) return true; - + if (_curMode == _mode) { if (!wasPrepared) _lock?.Prepare(); @@ -287,12 +287,19 @@ namespace nadena.dev.modular_avatar.core.armature_lock _lock?.Dispose(); _lock = null; -#if UNITY_EDITOR - AssemblyReloadEvents.beforeAssemblyReload -= Dispose; -#endif + #if UNITY_EDITOR + AssemblyReloadEvents.beforeAssemblyReload -= OnDomainUnload; + #endif _controllers.Remove(_mama); UpdateActive = false; } + + private void OnDomainUnload() + { + // Unity 2019 does not call deferred callbacks before domain unload completes, + // so we need to make sure to immediately destroy all our TransformAccessArrays. + DeferDestroy.DestroyImmediate(this); + } } } \ No newline at end of file diff --git a/Runtime/ArmatureAwase/BidirectionalArmatureLock.cs b/Runtime/ArmatureAwase/BidirectionalArmatureLock.cs index 46bca0b9..68e07533 100644 --- a/Runtime/ArmatureAwase/BidirectionalArmatureLock.cs +++ b/Runtime/ArmatureAwase/BidirectionalArmatureLock.cs @@ -13,7 +13,7 @@ using UnityEngine.Jobs; namespace nadena.dev.modular_avatar.core.armature_lock { - internal class BidirectionalArmatureLock : IDisposable, IArmatureLock + internal class BidirectionalArmatureLock : ArmatureLock, IDisposable { private bool _disposed; private TransformAccessArray _baseBoneAccess, _mergeBoneAccess; @@ -129,7 +129,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock } } - public void Dispose() + public override void Dispose() { if (_disposed) return; @@ -148,12 +148,12 @@ namespace nadena.dev.modular_avatar.core.armature_lock _disposed = true; } - public void Prepare() + public override void Prepare() { if (_disposed) return; - + LastOp.Complete(); - + WroteAny.Value = 0; var readBase = new ReadBone() @@ -175,7 +175,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock private bool CheckConsistency() { if (_disposed) return false; - + // Check parents haven't changed for (int i = 0; i < _baseBones.Length; i++) { @@ -194,16 +194,16 @@ namespace nadena.dev.modular_avatar.core.armature_lock return true; } - public bool IsStable() + public override bool IsStable() { Prepare(); if (!CheckConsistency()) return false; LastPrepare.Complete(); - + return WroteAny.Value == 0; } - public LockResult Execute() + public override LockResult Execute() { if (!CheckConsistency()) return LockResult.Failed; diff --git a/Runtime/ArmatureAwase/DeferDestroy.cs b/Runtime/ArmatureAwase/DeferDestroy.cs index 0111edb0..853c5cfe 100644 --- a/Runtime/ArmatureAwase/DeferDestroy.cs +++ b/Runtime/ArmatureAwase/DeferDestroy.cs @@ -1,6 +1,7 @@ #region using System; +using UnityEngine; #endregion @@ -8,8 +9,29 @@ namespace nadena.dev.modular_avatar.core.armature_lock { internal static class DeferDestroy { + private static bool _immediate = false; + + internal static void DestroyImmediate(IDisposable obj) + { + var oldValue = _immediate; + _immediate = true; + try + { + obj.Dispose(); + } + finally + { + _immediate = oldValue; + } + } + internal static void DeferDestroyObj(IDisposable obj) { + if (_immediate) + { + obj.Dispose(); + return; + } #if UNITY_EDITOR UnityEditor.EditorApplication.delayCall += () => obj.Dispose(); #endif diff --git a/Runtime/ArmatureAwase/IArmatureLock.cs b/Runtime/ArmatureAwase/IArmatureLock.cs deleted file mode 100644 index b6f32165..00000000 --- a/Runtime/ArmatureAwase/IArmatureLock.cs +++ /dev/null @@ -1,11 +0,0 @@ -using System; - -namespace nadena.dev.modular_avatar.core.armature_lock -{ - internal interface IArmatureLock : IDisposable - { - void Prepare(); - LockResult Execute(); - bool IsStable(); - } -} \ No newline at end of file diff --git a/Runtime/ArmatureAwase/OnewayArmatureLock.cs b/Runtime/ArmatureAwase/OnewayArmatureLock.cs index db96d9d9..63fbc9a4 100644 --- a/Runtime/ArmatureAwase/OnewayArmatureLock.cs +++ b/Runtime/ArmatureAwase/OnewayArmatureLock.cs @@ -16,7 +16,7 @@ using UnityEditor; namespace nadena.dev.modular_avatar.core.armature_lock { - internal class OnewayArmatureLock : IDisposable, IArmatureLock + internal class OnewayArmatureLock : ArmatureLock, IDisposable { struct BoneStaticData { @@ -189,9 +189,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock _baseBonesAccessor = new TransformAccessArray(_baseBones); _mergeBonesAccessor = new TransformAccessArray(_mergeBones); -#if UNITY_EDITOR - AssemblyReloadEvents.beforeAssemblyReload += Dispose; -#endif + EnableAssemblyReloadCallback = true; } private bool SmallScale(Vector3 scale) @@ -201,15 +199,15 @@ namespace nadena.dev.modular_avatar.core.armature_lock return (scale.x < epsilon || scale.y < epsilon || scale.z < epsilon); } - public void Prepare() + public override void Prepare() { if (_disposed) return; - + LastOp.Complete(); _baseBonesAccessor.SetTransforms(_baseBones); _mergeBonesAccessor.SetTransforms(_mergeBones); - + _fault.Value = 0; _wroteAny.Value = 0; @@ -255,7 +253,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock return true; } - public bool IsStable() + public override bool IsStable() { Prepare(); if (!CheckConsistency()) return false; @@ -269,10 +267,10 @@ namespace nadena.dev.modular_avatar.core.armature_lock /// Executes the armature lock job. /// /// True if successful, false if cached data was invalidated and needs recreating - public LockResult Execute() + public override LockResult Execute() { if (!CheckConsistency()) return LockResult.Failed; - + var commit = new WriteBone() { _fault = _fault, @@ -296,10 +294,10 @@ namespace nadena.dev.modular_avatar.core.armature_lock } } - public void Dispose() + public override void Dispose() { if (_disposed) return; - + LastOp.Complete(); _boneStaticData.Dispose(); _mergeSavedState.Dispose(); @@ -312,9 +310,7 @@ namespace nadena.dev.modular_avatar.core.armature_lock DeferDestroy.DeferDestroyObj(_mergeBonesAccessor); _disposed = true; -#if UNITY_EDITOR - AssemblyReloadEvents.beforeAssemblyReload -= Dispose; -#endif + EnableAssemblyReloadCallback = false; } } } \ No newline at end of file diff --git a/UnitTests~/BlendshapeSyncTests/BlendshapeSyncIntegrationTest.prefab b/UnitTests~/BlendshapeSyncTests/BlendshapeSyncIntegrationTest.prefab index 46e1bb93..55b1bb32 100644 --- a/UnitTests~/BlendshapeSyncTests/BlendshapeSyncIntegrationTest.prefab +++ b/UnitTests~/BlendshapeSyncTests/BlendshapeSyncIntegrationTest.prefab @@ -26,19 +26,18 @@ Transform: m_PrefabInstance: {fileID: 0} m_PrefabAsset: {fileID: 0} m_GameObject: {fileID: 3825275463613500755} - serializedVersion: 2 m_LocalRotation: {x: 0, y: 0, z: 0, w: 1} m_LocalPosition: {x: 0.023681391, y: 1.0559628, z: -0.6872994} m_LocalScale: {x: 1, y: 1, z: 1} - m_ConstrainProportionsScale: 0 m_Children: - {fileID: 3646968714803193661} - {fileID: 3646968713996568948} m_Father: {fileID: 0} + m_RootOrder: 0 m_LocalEulerAnglesHint: {x: 0, y: 0, z: 0} --- !u!95 &3825275463613500750 Animator: - serializedVersion: 5 + serializedVersion: 3 m_ObjectHideFlags: 0 m_CorrespondingSourceObject: {fileID: 0} m_PrefabInstance: {fileID: 0} @@ -51,12 +50,10 @@ Animator: m_UpdateMode: 0 m_ApplyRootMotion: 0 m_LinearVelocityBlending: 0 - m_StabilizeFeet: 0 m_WarningMessage: m_HasTransformHierarchy: 1 m_AllowConstantClipSamplingOptimization: 1 - m_KeepAnimatorStateOnDisable: 0 - m_WriteDefaultValuesOnDisable: 0 + m_KeepAnimatorControllerStateOnDisable: 0 --- !u!114 &3825275463613500753 MonoBehaviour: m_ObjectHideFlags: 0 @@ -325,12 +322,40 @@ MonoBehaviour: contentType: 0 assetBundleUnityVersion: fallbackStatus: 0 +--- !u!114 &3825275463971368602 +MonoBehaviour: + m_ObjectHideFlags: 0 + m_CorrespondingSourceObject: {fileID: 0} + m_PrefabInstance: {fileID: 0} + m_PrefabAsset: {fileID: 0} + m_GameObject: {fileID: 4167925416990528462} + m_Enabled: 1 + m_EditorHideFlags: 0 + m_Script: {fileID: 11500000, guid: 6fd7cab7d93b403280f2f9da978d8a4f, type: 3} + m_Name: + m_EditorClassIdentifier: + Bindings: + - ReferenceMesh: + referencePath: BaseMesh + Blendshape: shape_0 + LocalBlendshape: shape_0_local + - ReferenceMesh: + referencePath: BaseMesh + Blendshape: shape_1 + LocalBlendshape: shape_1 + - ReferenceMesh: + referencePath: MissingMesh + Blendshape: missing_mesh_shape + LocalBlendshape: missing_mesh_shape + - ReferenceMesh: + referencePath: + Blendshape: missing_mesh_shape_2 + LocalBlendshape: missing_mesh_shape_2 --- !u!1001 &3825275463173128406 PrefabInstance: m_ObjectHideFlags: 0 serializedVersion: 2 m_Modification: - serializedVersion: 3 m_TransformParent: {fileID: 3825275463613500751} m_Modifications: - target: {fileID: -8679921383154817045, guid: 14ac2ad30c5d3444ca37f76cea5a7047, @@ -429,9 +454,6 @@ PrefabInstance: value: BaseMesh objectReference: {fileID: 0} m_RemovedComponents: [] - m_RemovedGameObjects: [] - m_AddedGameObjects: [] - m_AddedComponents: [] m_SourcePrefab: {fileID: 100100000, guid: 14ac2ad30c5d3444ca37f76cea5a7047, type: 3} --- !u!4 &3646968714803193661 stripped Transform: @@ -444,7 +466,6 @@ PrefabInstance: m_ObjectHideFlags: 0 serializedVersion: 2 m_Modification: - serializedVersion: 3 m_TransformParent: {fileID: 3825275463613500751} m_Modifications: - target: {fileID: -8679921383154817045, guid: 14ac2ad30c5d3444ca37f76cea5a7047, @@ -548,52 +569,16 @@ PrefabInstance: value: SyncedMesh objectReference: {fileID: 0} m_RemovedComponents: [] - m_RemovedGameObjects: [] - m_AddedGameObjects: [] - m_AddedComponents: - - targetCorrespondingSourceObject: {fileID: 919132149155446097, guid: 14ac2ad30c5d3444ca37f76cea5a7047, - type: 3} - insertIndex: -1 - addedObject: {fileID: 3825275463971368602} m_SourcePrefab: {fileID: 100100000, guid: 14ac2ad30c5d3444ca37f76cea5a7047, type: 3} ---- !u!4 &3646968713996568948 stripped -Transform: - m_CorrespondingSourceObject: {fileID: -8679921383154817045, guid: 14ac2ad30c5d3444ca37f76cea5a7047, - type: 3} - m_PrefabInstance: {fileID: 3825275463971368607} - m_PrefabAsset: {fileID: 0} --- !u!1 &4167925416990528462 stripped GameObject: m_CorrespondingSourceObject: {fileID: 919132149155446097, guid: 14ac2ad30c5d3444ca37f76cea5a7047, type: 3} m_PrefabInstance: {fileID: 3825275463971368607} m_PrefabAsset: {fileID: 0} ---- !u!114 &3825275463971368602 -MonoBehaviour: - m_ObjectHideFlags: 0 - m_CorrespondingSourceObject: {fileID: 0} - m_PrefabInstance: {fileID: 0} +--- !u!4 &3646968713996568948 stripped +Transform: + m_CorrespondingSourceObject: {fileID: -8679921383154817045, guid: 14ac2ad30c5d3444ca37f76cea5a7047, + type: 3} + m_PrefabInstance: {fileID: 3825275463971368607} m_PrefabAsset: {fileID: 0} - m_GameObject: {fileID: 4167925416990528462} - m_Enabled: 1 - m_EditorHideFlags: 0 - m_Script: {fileID: 11500000, guid: 6fd7cab7d93b403280f2f9da978d8a4f, type: 3} - m_Name: - m_EditorClassIdentifier: - Bindings: - - ReferenceMesh: - referencePath: BaseMesh - Blendshape: shape_0 - LocalBlendshape: shape_0_local - - ReferenceMesh: - referencePath: BaseMesh - Blendshape: shape_1 - LocalBlendshape: shape_1 - - ReferenceMesh: - referencePath: MissingMesh - Blendshape: missing_mesh_shape - LocalBlendshape: missing_mesh_shape - - ReferenceMesh: - referencePath: - Blendshape: missing_mesh_shape_2 - LocalBlendshape: missing_mesh_shape_2