New AvatarMask is created and leaked each time SetMask is called with null as argument

[Shelim]

Environment

• Animancer Version Number: 7.4.2
• Animancer Pro or Lite: Pro
• Unity Version: 2021.3.23f1
• Platform: Windows

Description

Each time SetMask is called with null argument, there is a code that creates a new AvatarMask (native resource!) and never destroys it. See file internal\Core\AnimancerPlayable.LayerList.cs, line 315:

if (mask == null)
    mask = new AvatarMask();

This mask is leaking 40 bytes per function call and never released memory. On mobile platform this can exhaust device RAM in several minutes.

Reproduction

Steps to reproduce the bug:

1. Call SetMask(null) each frame (directly or indirectly)
2. Observe memory leak in Memory Analyser

[Shelim]

Here is output from Unity memory analyser:

[KybernetikGames]

That check was actually only there because passing in null caused Unity 2018 to crash, but now that's no longer necessary so I've removed it for the next version of Animancer and added this to the comment:

            /// <remarks>
            /// Don't call this method repeatedly with the same mask unless you have modified it. It doesn't check if
            /// the mask is the same so repeatedly assigning the same thing will simply waste performance.
            /// <para></para>
            /// The `mask` can't be <c>null</c> so if you want to clear a mask, make a new <see cref="AvatarMask"/> and
            /// store it in a <c>static</c> field so you can pass it into this method when necessary.
            /// </remarks>

[Shelim]

Woah, thanks for quick answer! And thanks for comment - it looks like I can squeeze a bit more performance by just checking if the mask was changed :)

[KybernetikGames]

I've actually changed my mind on this issue. It would be much more convenient if you were allowed to have a serialized AvatarMask field and leave it blank or copy the mask from one layer to another without needing to null check it yourself.

So I've changed the method to this:

            private static AvatarMask _DefaultMask;

            /// <summary>[Pro-Only]
            /// Sets an <see cref="AvatarMask"/> to determine which bones the layer at the specified index will affect.
            /// </summary>
            /// <remarks>
            /// Don't call this method repeatedly with the same mask unless you have modified it. It doesn't check if
            /// the mask is the same so repeatedly assigning the same thing will simply waste performance.
            /// </remarks>
            public virtual void SetMask(int index, AvatarMask mask)
            {
                var layer = this[index];

                if (mask == null)
                {
                    // If the existing mask was already null, do nothing.
                    // If it was destroyed, we still need to continue and set it to the default.
                    if (ReferenceEquals(layer._Mask, null))
                        return;

                    if (ReferenceEquals(_DefaultMask, null))
                        _DefaultMask = new();

                    mask = _DefaultMask;
                }

                // Don't check if the same mask was already assigned because it might have been modified.
                layer._Mask = mask;

                LayerMixer.SetLayerMaskFromAvatarMask((uint)index, mask);
            }

[Shelim]

Looks much better, thanks!

[GameDevBox]

I've actually changed my mind on this issue. It would be much more convenient if you were allowed to have a serialized AvatarMask field and leave it blank or copy the mask from one layer to another without needing to null check it yourself.

So I've changed the method to this:

            private static AvatarMask _DefaultMask;

            /// <summary>[Pro-Only]
            /// Sets an <see cref="AvatarMask"/> to determine which bones the layer at the specified index will affect.
            /// </summary>
            /// <remarks>
            /// Don't call this method repeatedly with the same mask unless you have modified it. It doesn't check if
            /// the mask is the same so repeatedly assigning the same thing will simply waste performance.
            /// </remarks>
            public virtual void SetMask(int index, AvatarMask mask)
            {
                var layer = this[index];

                if (mask == null)
                {
                    // If the existing mask was already null, do nothing.
                    // If it was destroyed, we still need to continue and set it to the default.
                    if (ReferenceEquals(layer._Mask, null))
                        return;

                    if (ReferenceEquals(_DefaultMask, null))
                        _DefaultMask = new();

                    mask = _DefaultMask;
                }

                // Don't check if the same mask was already assigned because it might have been modified.
                layer._Mask = mask;

                LayerMixer.SetLayerMaskFromAvatarMask((uint)index, mask);
            }

hi, I put this code in Animancer 7.4.2 but when I try to build the game I got two errors related to these lines.

if (ReferenceEquals(layer._Mask, null))

Assets\Plugins\Animancer\Internal\Core\AnimancerPlayable.LayerList.cs(321,47): error CS1061: 'AnimancerLayer' does not contain a definition for '_Mask' and no accessible extension method '_Mask' accepting a first argument of type 'AnimancerLayer' could be found (are you missing a using directive or an assembly reference?)

layer._Mask = mask;

Assets\Plugins\Animancer\Internal\Core\AnimancerPlayable.LayerList.cs(331,23): error CS1061: 'AnimancerLayer' does not contain a definition for '_Mask' and no accessible extension method '_Mask' accepting a first argument of type 'AnimancerLayer' could be found (are you missing a using directive or an assembly reference?)

what could cause this should I revert the changes?

[KybernetikGames]

You'll also need to remove the #if around the AnimancerLayer._Mask field so it doesn't get compiled out of builds.

[GameDevBox]

Thanks!

[KybernetikGames]

Animancer v8.0 is now available for Alpha Testing and includes this fix..

[KybernetikGames]

Animancer v8.0 is now fully released.