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

feat(animation): animations registry #7528

Merged
merged 31 commits into from
Jan 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c19b2c8
todos
ShaMan123 Dec 1, 2021
d9fb32d
feat(animation): running animations registry
ShaMan123 Dec 1, 2021
3284141
test(animation): running animations
ShaMan123 Dec 1, 2021
6f88020
lint
ShaMan123 Dec 1, 2021
8b1d422
Update animate.js
ShaMan123 Dec 1, 2021
7e55962
Update animate.js
ShaMan123 Dec 1, 2021
5793ced
fix(): cancel all
ShaMan123 Dec 1, 2021
0466f25
perf
ShaMan123 Dec 1, 2021
809ff98
typo
ShaMan123 Dec 5, 2021
af7cdc3
revert todos c19b2c8d
ShaMan123 Dec 5, 2021
456a360
Update animate.js
ShaMan123 Dec 5, 2021
9f4bbe6
Update animate.js
ShaMan123 Dec 5, 2021
2c5478f
Update animate.js
ShaMan123 Dec 5, 2021
16a4a40
Update animation.js
ShaMan123 Dec 5, 2021
e4aaa5b
current values
ShaMan123 Dec 6, 2021
f798f6b
tests(): animation context current state
ShaMan123 Dec 6, 2021
d761584
jsdoc
ShaMan123 Dec 6, 2021
d4c5606
ci(): lint
ShaMan123 Dec 6, 2021
d3595b6
Update canvas.class.js
ShaMan123 Jan 19, 2022
3015a2d
add `target` property to object's animations
ShaMan123 Jan 19, 2022
54a4d0f
support byTarget
ShaMan123 Jan 19, 2022
686f91f
object dispose
ShaMan123 Jan 19, 2022
a0f7e40
Update animate_color.js
ShaMan123 Jan 19, 2022
18f37ec
Update animation.js
ShaMan123 Jan 19, 2022
06c8243
Update canvas.js
ShaMan123 Jan 19, 2022
7ec9c3a
Update object.js
ShaMan123 Jan 19, 2022
d17e28f
imperative abort return context
ShaMan123 Jan 19, 2022
0695460
remove this in 5.0
ShaMan123 Jan 19, 2022
7a406e8
Create outputA.txt
ShaMan123 Jan 19, 2022
865be3c
**BREAKING** chainable methods to return animation abort fuction
ShaMan123 Jan 19, 2022
bbfd7b3
cleanup
ShaMan123 Jan 21, 2022
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
1 change: 1 addition & 0 deletions src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,7 @@
*/
dispose: function () {
var wrapper = this.wrapperEl;
fabric.runningAnimations.cancelAll();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be executed per canvas.
EIther you pass in the canvas as argument and cancelAll becomes a cancelAll by canvas, or in an application with 2 canvases this would stop everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.
When an objects animates does it have to be attached to a canvas? I guess not. So I'm not sure how to do what you suggest.
I can add a canvas property to the animation context object but what if there's no canvas set to the object??
Or maybe we should iterate over all running animations and check at that moment who is attached to the canvas arg and cancel that?

this.removeListeners();
wrapper.removeChild(this.upperCanvasEl);
wrapper.removeChild(this.lowerCanvasEl);
Expand Down
37 changes: 16 additions & 21 deletions src/mixins/animation.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
* @param {Object} [callbacks] Callbacks object with optional "onComplete" and/or "onChange" properties
* @param {Function} [callbacks.onComplete] Invoked on completion
* @param {Function} [callbacks.onChange] Invoked on every step of animation
* @return {fabric.Canvas} thisArg
* @chainable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe i m wrong, but is chainability still cool to do?
I understand it saves typing and as a result bits in the final bundle.
But i find having half methods chainable and half not is weird. Maybe is something we should drop in the rewrite.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Chaining is not worth the weirdness.
I say we drop it.

Copy link
Contributor Author

@ShaMan123 ShaMan123 Jan 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it is less breaking to have void return values that change (for future impl)

* @return {fabric.AnimationContext} context
*/
fxCenterObjectH: function (object, callbacks) {
callbacks = callbacks || { };
Expand All @@ -24,7 +23,8 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
onChange = callbacks.onChange || empty,
_this = this;

fabric.util.animate({
return fabric.util.animate({
target: this,
startValue: object.left,
endValue: this.getCenter().left,
duration: this.FX_DURATION,
Expand All @@ -38,8 +38,6 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
onComplete();
}
});

return this;
},

/**
Expand All @@ -48,8 +46,7 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
* @param {Object} [callbacks] Callbacks object with optional "onComplete" and/or "onChange" properties
* @param {Function} [callbacks.onComplete] Invoked on completion
* @param {Function} [callbacks.onChange] Invoked on every step of animation
* @return {fabric.Canvas} thisArg
* @chainable
* @return {fabric.AnimationContext} context
*/
fxCenterObjectV: function (object, callbacks) {
callbacks = callbacks || { };
Expand All @@ -59,7 +56,8 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
onChange = callbacks.onChange || empty,
_this = this;

fabric.util.animate({
return fabric.util.animate({
target: this,
startValue: object.top,
endValue: this.getCenter().top,
duration: this.FX_DURATION,
Expand All @@ -73,8 +71,6 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
onComplete();
}
});

return this;
},

/**
Expand All @@ -83,8 +79,7 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
* @param {Object} [callbacks] Callbacks object with optional "onComplete" and/or "onChange" properties
* @param {Function} [callbacks.onComplete] Invoked on completion
* @param {Function} [callbacks.onChange] Invoked on every step of animation
* @return {fabric.Canvas} thisArg
* @chainable
* @return {fabric.AnimationContext} context
*/
fxRemove: function (object, callbacks) {
callbacks = callbacks || { };
Expand All @@ -94,7 +89,8 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
onChange = callbacks.onChange || empty,
_this = this;

fabric.util.animate({
return fabric.util.animate({
target: this,
startValue: object.opacity,
endValue: 0,
duration: this.FX_DURATION,
Expand All @@ -108,8 +104,6 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
onComplete();
}
});

return this;
}
});

Expand All @@ -120,7 +114,7 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
* @param {Number|Object} value Value to animate property to (if string was given first) or options object
* @return {fabric.Object} thisArg
* @tutorial {@link http://fabricjs.com/fabric-intro-part-2#animation}
* @chainable
* @return {fabric.AnimationContext | fabric.AnimationContext[]} animation context (or an array if passed multiple properties)
*
* As object — multiple properties
*
Expand All @@ -133,22 +127,22 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
* object.animate('left', { duration: ... });
*
*/
animate: function() {
animate: function () {
if (arguments[0] && typeof arguments[0] === 'object') {
var propsToAnimate = [], prop, skipCallbacks;
var propsToAnimate = [], prop, skipCallbacks, out = [];
for (prop in arguments[0]) {
propsToAnimate.push(prop);
}
for (var i = 0, len = propsToAnimate.length; i < len; i++) {
prop = propsToAnimate[i];
skipCallbacks = i !== len - 1;
this._animate(prop, arguments[0][prop], arguments[1], skipCallbacks);
out.push(this._animate(prop, arguments[0][prop], arguments[1], skipCallbacks));
}
return out;
}
else {
this._animate.apply(this, arguments);
return this._animate.apply(this, arguments);
}
return this;
},

/**
Expand Down Expand Up @@ -196,6 +190,7 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
}

var _options = {
target: this,
startValue: options.from,
endValue: to,
byValue: options.by,
Expand Down
14 changes: 3 additions & 11 deletions src/mixins/object_straightening.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
/**
* Straightens an object (rotating it from current angle to one of 0, 90, 180, 270, etc. depending on which is closer)
* @return {fabric.Object} thisArg
* @chainable
*/
straighten: function() {
this.rotate(this._getAngleValueForStraighten());
return this;
},

/**
Expand All @@ -28,7 +26,6 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
* @param {Function} [callbacks.onComplete] Invoked on completion
* @param {Function} [callbacks.onChange] Invoked on every step of animation
* @return {fabric.Object} thisArg
* @chainable
*/
fxStraighten: function(callbacks) {
callbacks = callbacks || { };
Expand All @@ -38,7 +35,8 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
onChange = callbacks.onChange || empty,
_this = this;

fabric.util.animate({
return fabric.util.animate({
target: this,
startValue: this.get('angle'),
endValue: this._getAngleValueForStraighten(),
duration: this.FX_DURATION,
Expand All @@ -51,8 +49,6 @@ fabric.util.object.extend(fabric.Object.prototype, /** @lends fabric.Object.prot
onComplete();
},
});

return this;
}
});

Expand All @@ -62,24 +58,20 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
* Straightens object, then rerenders canvas
* @param {fabric.Object} object Object to straighten
* @return {fabric.Canvas} thisArg
* @chainable
*/
straightenObject: function (object) {
object.straighten();
this.requestRenderAll();
return this;
},

/**
* Same as {@link fabric.Canvas.prototype.straightenObject}, but animated
* @param {fabric.Object} object Object to straighten
* @return {fabric.Canvas} thisArg
* @chainable
*/
fxStraightenObject: function (object) {
object.fxStraighten({
return object.fxStraighten({
onChange: this.requestRenderAllBound
});
return this;
}
});
3 changes: 2 additions & 1 deletion src/shapes/image.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@
/**
* Delete textures, reference to elements and eventually JSDOM cleanup
*/
dispose: function() {
dispose: function () {
this.callSuper('dispose');
this.removeTexture(this.cacheKey);
this.removeTexture(this.cacheKey + '_filtered');
this._cacheContext = undefined;
Expand Down
7 changes: 7 additions & 0 deletions src/shapes/object.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,13 @@
if (this.globalCompositeOperation) {
ctx.globalCompositeOperation = this.globalCompositeOperation;
}
},

/**
* cancel instance's running animations
*/
dispose: function () {
fabric.runningAnimations.cancelByTarget(this);
}
});

Expand Down
18 changes: 1 addition & 17 deletions src/static_canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,6 @@
/**
* Centers object horizontally in the canvas
* @param {fabric.Object} object Object to center horizontally
* @return {fabric.Canvas} thisArg
*/
centerObjectH: function (object) {
return this._centerObject(object, new fabric.Point(this.getCenter().left, object.getCenterPoint().y));
Expand All @@ -1047,8 +1046,6 @@
/**
* Centers object vertically in the canvas
* @param {fabric.Object} object Object to center vertically
* @return {fabric.Canvas} thisArg
* @chainable
*/
centerObjectV: function (object) {
return this._centerObject(object, new fabric.Point(object.getCenterPoint().x, this.getCenter().top));
Expand All @@ -1057,8 +1054,6 @@
/**
* Centers object vertically and horizontally in the canvas
* @param {fabric.Object} object Object to center vertically and horizontally
* @return {fabric.Canvas} thisArg
* @chainable
*/
centerObject: function(object) {
var center = this.getCenter();
Expand All @@ -1069,8 +1064,6 @@
/**
* Centers object vertically and horizontally in the viewport
* @param {fabric.Object} object Object to center vertically and horizontally
* @return {fabric.Canvas} thisArg
* @chainable
*/
viewportCenterObject: function(object) {
var vpCenter = this.getVpCenter();
Expand All @@ -1081,20 +1074,15 @@
/**
* Centers object horizontally in the viewport, object.top is unchanged
* @param {fabric.Object} object Object to center vertically and horizontally
* @return {fabric.Canvas} thisArg
* @chainable
*/
viewportCenterObjectH: function(object) {
var vpCenter = this.getVpCenter();
this._centerObject(object, new fabric.Point(vpCenter.x, object.getCenterPoint().y));
return this;
return this._centerObject(object, new fabric.Point(vpCenter.x, object.getCenterPoint().y));
},

/**
* Centers object Vertically in the viewport, object.top is unchanged
* @param {fabric.Object} object Object to center vertically and horizontally
* @return {fabric.Canvas} thisArg
* @chainable
*/
viewportCenterObjectV: function(object) {
var vpCenter = this.getVpCenter();
Expand All @@ -1105,7 +1093,6 @@
/**
* Calculate the point in canvas that correspond to the center of actual viewport.
* @return {fabric.Point} vpCenter, viewport center
* @chainable
*/
getVpCenter: function() {
var center = this.getCenter(),
Expand All @@ -1117,14 +1104,11 @@
* @private
* @param {fabric.Object} object Object to center
* @param {fabric.Point} center Center point
* @return {fabric.Canvas} thisArg
* @chainable
*/
_centerObject: function(object, center) {
object.setPositionByOrigin(center, 'center', 'center');
object.setCoords();
this.renderOnAddRemove && this.requestRenderAll();
return this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would like to limit breaking changes in 5. Those are not part of the animation changes. What is the reason to remove chainability now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's as you said. I thought it's too weird to have the animated version not chainable while the regular one is.

},

/**
Expand Down
Loading