MorphTargetManager performence issue

I am handling an animation with N morph target tracks.

In this case, reducing the number of calls to _syncActiveTargets will have a significant impact on performance.

Before #16014, it was possible to reduce the number of calls to _syncActiveTargets to once per animation processing using areUpdatesFrozen.

The code will look something like this:

// This prevents _syncActiveTargets from being called on each iteration of the loop.
morphTargetManager.areUpdatesFrozen = true;

for (const [morphIndex, morphWeight] in evaluatedAnimations) {
    applyAnimation(morphIndex, morphWeight);
}

morphTargetManager.areUpdatesFrozen = false;

However, after #16014, optimizing the number of calls to _syncActiveTargets with areUpdatesFrozen is no longer possible, because we set _mustSynchronize to true at the point where areUpdatesFrozen becomes false, which causes us to run the slow task of generating morph target texures.

To recap the problem

A common use case for the Morph Target Manager is to mix multiple Morphs in the same time.

  1. In this case, you would use a loop to update one morph weight on each iteration.
for (const [morphIndex, morphWeight] in evaluatedAnimations) {
    applyAnimation(morphIndex, morphWeight);
}
  1. On each iteration, the MorphTarget then fires the onInfluenceChanged observer, which causes the _syncActiveTargets function to run.
    If you have 10 morphs on your animation track, _syncActiveTargets will be executed 10 times.
  1. Eventually, _syncActiveTargets will run N times, but the only thing that really matters is the result of the last run; the other N - 1 runs will be for nothing.

To fix this, it looks like we need to apply a dirtyFlag to _syncActiveTargets to treat it with lazy evaluation. Before #16014, I optimized animation evaluation by declaratively using areUpdatesFrozen, but that doesnā€™t seem like a good idea either.

@Evgeni_Popov

1 Like

I will leave this one to @Evgeni_Popov next week when he ll be back from a well deserved vacation :slight_smile:

1 Like

I think the fix is to simply not set this._mustSynchronize = true when updates are not frozen anymore:

  • this flag will be true if any changes requiring a texture recreation have been done while udpates were frozen.
  • if not, the flag will be false and we wonā€™t recreate the texture when updates are allowed again.

Hereā€™s the PR:

Let me know if it works for you.

1 Like

Thanks, this seems to solve the problem in my case, but for good performance in more cases where we update morph target influences, we need to add more complex conditions to the flag so that _syncActiveTargets can be lazy evaluated at render time instead of just using the current _mustSynchronize dirty flag.

It looks to me that setting areUpdatesFrozen = true / updating target influences / setting areUpdatesFrozen = false is optimal (when the PR is merged)? Because of the call to areUpdatesFrozen = true, updating influences wonā€™t call _syncActiveTargets. _syncActiveTargets will be called (once) only when setting areUpdatesFrozen = false.

Iā€™ve pictured a scenario where multiple animation systems each try to use that method for optimal performance, and it seems to work fine in this case.

In code, it looks like this

// The code for each system is assumed to be library-embedded and
// not modifiable at the end-user(developer) level.

// animation system1 from some babylon js related npm lib or something...
function animate1(...) {
    // some animation evaluations...

    morphTargetManager.areUpdatesFrozen = true;
    for (const [morphIndex, morphWeight] in evaluatedAnimations) {
        applyAnimation(morphIndex, morphWeight);
    }
    morphTargetManager.areUpdatesFrozen = false;
}

// animation system2 from some babylon js related npm lib or something...
function animate2(...) {
    // some animation evaluations...

    morphTargetManager.areUpdatesFrozen = true;
    for (const [morphIndex, morphWeight] in evaluatedAnimations) {
        applyAnimation(morphIndex, morphWeight);
    }
    morphTargetManager.areUpdatesFrozen = false;
}

// Users can use animation systems from multiple libraries simultaneously.

// However, this will cause `areUpdatesFrozen = false` to be executed once
// in each animate function, resulting in `MorphTargetManager._syncActiveTargets`
// being executed twice.
animate1(mesh);
animate2(mesh);

// If users set areUpdatesFrozen to true before using the animation system
// and set it to false after all animation is evaluated,
// `MorphTargetManager._syncActiveTargets` will only run once.
morphTargetManager.areUpdatesFrozen = true;
animate1(mesh);
animate2(mesh);
morphTargetManager.areUpdatesFrozen = false;

I think this is good enough for me.

1 Like

I just applied and tested the 7.43.0 release and it seems to work fine.

1 Like

Found an issue: MorphTarget animation does not work properly when numMaxInfluencers is not specified, because the MorphTargetManager.synchronize method is not called when areUpdatesFrozen = false.

Here is a PG that reproduces the issue.
Please check the comments

1 Like

What are you doing when areUpdatesFrozen == true that would require synchronize() to be called when areUpdatesFrozen is reset to false? It seems we miss to set this._mustSynchronize = true; somewhere, but itā€™s hard to debug with your PG as everything happens in the mmd library.

1 Like

I apologize for the PG being quite rough in my haste to share.

Hereā€™s a PG that reproduces the problem with few elements.

Notably, this issue is not reproduced in PBRMaterials or NodeMaterials, only in StandardMaterials.

And the issue was caused by the meshā€™s dirty flag not being set properly. I wrote more about it in PGā€™s comment.

2 Likes

Donā€™t apologize, you did a lot to fix morph implementation, and your latest PG is exactly what I needed to fix the bug!

Hereā€™s the PR:

Note that you must use a setTimeout in your repro because the bug wonā€™t show up if Material.isReadyForSubMesh is called after areUpdatesFrozen is set to true. By introducing the delay, you make sure that this method is called one time before updates are frozen. I think the bug doesnā€™t show up with a PBR material because it takes more time for this material to compile, but if you set a timeout like 16ms, for eg, you will see the same problem.

2 Likes

Thank you :slight_smile: Iā€™ll test it out and let you know if there are any other issues on morph target.

+The issue seems to be fixed in version 7.44.0.