Skip to content

Commit

Permalink
fix(StaticCanvas): cancelling animations on dispose (#9361)
Browse files Browse the repository at this point in the history
  • Loading branch information
ShaMan123 authored Sep 22, 2023
1 parent d217b0f commit ef27fcd
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [next]

- fix(StaticCanvas): disposing animations [#9361](https://github.com/fabricjs/fabric.js/pull/9361)
- fix(IText): cursor width under group [#9341](https://github.com/fabricjs/fabric.js/pull/9341)
- TS(Canvas): constructor optional el [#9348](https://github.com/fabricjs/fabric.js/pull/9348)

Expand Down
2 changes: 2 additions & 0 deletions src/canvas/StaticCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
cancelAnimFrame,
requestAnimFrame,
} from '../util/animation/AnimationFrameProvider';
import { runningAnimations } from '../util/animation/AnimationRegistry';
import { uid } from '../util/internals/uid';
import { createCanvasElement, toDataURL } from '../util/misc/dom';
import { invertTransform, transformPoint } from '../util/misc/matrix';
Expand Down Expand Up @@ -1440,6 +1441,7 @@ export class StaticCanvas<
dispose() {
!this.disposed &&
this.elements.cleanupDOM({ width: this.width, height: this.height });
runningAnimations.cancelByCanvas(this);
this.disposed = true;
return new Promise<boolean>((resolve, reject) => {
const task = () => {
Expand Down
13 changes: 7 additions & 6 deletions src/util/animation/AnimationRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Canvas } from '../../canvas/Canvas';
import type { StaticCanvas } from '../../canvas/StaticCanvas';
import type { FabricObject } from '../../shapes/Object/FabricObject';
import type { AnimationBase } from './AnimationBase';

Expand All @@ -25,17 +25,18 @@ class AnimationRegistry extends Array<AnimationBase> {
}

/**
* Cancel all running animations attached to a Canvas on the next frame
* @param {Canvas} canvas
* Cancel all running animations attached to a canvas on the next frame
* @param {StaticCanvas} canvas
*/
cancelByCanvas(canvas: Canvas) {
cancelByCanvas(canvas: StaticCanvas) {
if (!canvas) {
return [];
}
const animations = this.filter(
(animation) =>
typeof animation.target === 'object' &&
(animation.target as FabricObject)?.canvas === canvas
animation.target === canvas ||
(typeof animation.target === 'object' &&
(animation.target as FabricObject)?.canvas === canvas)
);
animations.forEach((animation) => animation.abort());
return animations;
Expand Down
27 changes: 27 additions & 0 deletions test/unit/canvas_dispose.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,33 @@ function assertCanvasDisposing(klass) {
});
animate();
});

QUnit.test('disposing during animation should cancel it by target', function (assert) {
const done = assert.async();
const canvas = new klass(null, { renderOnAddRemove: false });
let called = 0;
const animate = () => fabric.util.animate({
target: canvas,
onChange() {
if (called === 1) {
assert.equal(fabric.runningAnimations[0].target, canvas, 'should register the animation by target');
canvas.dispose().then(() => {
assert.deepEqual(fabric.runningAnimations, [], 'should cancel the animation');
done();
});
assert.ok(canvas.disposed, 'should flag `disposed`');
}
called++;
canvas.contextTopDirty = true;
canvas.hasLostContext = true;
canvas.renderAll();
},
onComplete() {
animate();
}
});
animate();
});
}

function testStaticCanvasDisposing() {
Expand Down

0 comments on commit ef27fcd

Please sign in to comment.