Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve how we delete massive group of animatables from animationgroup #13641

Merged
merged 5 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions packages/dev/core/src/Animations/animatable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export class Animatable {
private _localDelayOffset: Nullable<number> = null;
private _pausedDelay: Nullable<number> = null;
private _manualJumpDelay: Nullable<number> = null;
private _runtimeAnimations = new Array<RuntimeAnimation>();
/** @hidden */
public _runtimeAnimations = new Array<RuntimeAnimation>();
private _paused = false;
private _scene: Scene;
private _speedRatio = 1;
Expand Down Expand Up @@ -319,9 +320,10 @@ export class Animatable {
/**
* Stop and delete the current animation
* @param animationName defines a string used to only stop some of the runtime animations instead of all
* @param targetMask - a function that determines if the animation should be stopped based on its target (all animations will be stopped if both this and animationName are empty)
* @param targetMask a function that determines if the animation should be stopped based on its target (all animations will be stopped if both this and animationName are empty)
* @param useGlobalSplice if true, the animatables will be removed by the caller of this function (false by default)
*/
public stop(animationName?: string, targetMask?: (target: any) => boolean): void {
public stop(animationName?: string, targetMask?: (target: any) => boolean, useGlobalSplice = false): void {
if (animationName || targetMask) {
const idx = this._scene._activeAnimatables.indexOf(this);

Expand All @@ -342,20 +344,20 @@ export class Animatable {
}

if (runtimeAnimations.length == 0) {
this._scene._activeAnimatables.splice(idx, 1);
if (!useGlobalSplice) {
this._scene._activeAnimatables.splice(idx, 1);
}
this._raiseOnAnimationEnd();
}
}
} else {
const index = this._scene._activeAnimatables.indexOf(this);

if (index > -1) {
this._scene._activeAnimatables.splice(index, 1);
const runtimeAnimations = this._runtimeAnimations;

for (let index = 0; index < runtimeAnimations.length; index++) {
runtimeAnimations[index].dispose();
if (!useGlobalSplice) {
this._scene._activeAnimatables.splice(index, 1);
}
this._runtimeAnimations.length = 0;

this._raiseOnAnimationEnd();
}
Expand Down
5 changes: 4 additions & 1 deletion packages/dev/core/src/Animations/animationGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,12 @@ export class AnimationGroup implements IDisposable {

const list = this._animatables.slice();
for (let index = 0; index < list.length; index++) {
list[index].stop();
list[index].stop(undefined, undefined, true);
}

// We will take care of removing all stopped animatables
this._scene._activeAnimatables = this._scene._activeAnimatables.filter((anim) => anim._runtimeAnimations.length > 0);

this._isStarted = false;

return this;
Expand Down
1 change: 1 addition & 0 deletions packages/dev/core/src/Animations/runtimeAnimation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ export class RuntimeAnimation {

/**
* Disposes of the runtime animation
* Note: No hard dispose should happen here as this method is skipped for performance reason (look at animatable.stop())
*/
public dispose(): void {
const index = this._animation.runtimeAnimations.indexOf(this);
Expand Down