From 7218e664da38fc91e413def8df9fdc345f1c32a1 Mon Sep 17 00:00:00 2001 From: Alan Cutter Date: Tue, 2 Aug 2016 11:46:20 +1000 Subject: [PATCH 1/5] optimiseAnimationMutations --- Gruntfile.js | 2 +- src/animation.js | 32 ++++++++++++++++++++++++++------ src/keyframe-effect.js | 1 + src/tick.js | 41 ++++++++++++++++++++++++++++------------- test/js/animation.js | 12 ++++++------ 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index aeeb406..62a2c69 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -237,7 +237,7 @@ module.exports = function(grunt) { configCallback(karmaConfig); karmaConfig.client.testharnessTests = require('./test/web-platform-tests-expectations.js'); karmaConfig.client.testharnessTests.testURLList = testFiles; - karmaConfig.proxies['/base/polyfill.js'] = '/base/' + task.target + '.min.js'; + karmaConfig.proxies['/base/polyfill.js'] = '/base/' + task.target + '.dev.js'; karmaConfig.files.push('test/karma-testharness-adapter.js'); var servedFiles = [ 'test/web-platform-tests/resources/**', diff --git a/src/animation.js b/src/animation.js index f1d21d8..58e3918 100644 --- a/src/animation.js +++ b/src/animation.js @@ -94,7 +94,7 @@ this._paused = true; } this._tickCurrentTime(newTime, true); - scope.invalidateEffects(); + scope.applyDirtiedAnimation(this); }, get startTime() { return this._startTime; @@ -107,7 +107,7 @@ return; this._startTime = newTime; this._tickCurrentTime((this._timeline.currentTime - this._startTime) * this.playbackRate); - scope.invalidateEffects(); + scope.applyDirtiedAnimation(this); }, get playbackRate() { return this._playbackRate; @@ -123,7 +123,7 @@ this._finishedFlag = false; this._idle = false; this._ensureAlive(); - scope.invalidateEffects(); + scope.applyDirtiedAnimation(this); } if (oldCurrentTime != null) { this.currentTime = oldCurrentTime; @@ -165,7 +165,7 @@ this._finishedFlag = false; this._idle = false; this._ensureAlive(); - scope.invalidateEffects(); + scope.applyDirtiedAnimation(this); }, pause: function() { if (!this._isFinished && !this._paused && !this._idle) { @@ -183,7 +183,7 @@ this.currentTime = this._playbackRate > 0 ? this._totalDuration : 0; this._startTime = this._totalDuration - this.currentTime; this._currentTimePending = false; - scope.invalidateEffects(); + scope.applyDirtiedAnimation(this); }, cancel: function() { if (!this._inEffect) @@ -197,7 +197,7 @@ this._effect._update(null); // effects are invalid after cancellation as the animation state // needs to un-apply. - scope.invalidateEffects(); + scope.applyDirtiedAnimation(this); }, reverse: function() { this.playbackRate *= -1; @@ -249,6 +249,26 @@ get _needsTick() { return (this.playState in {'pending': 1, 'running': 1}) || !this._finishedFlag; }, + _targetAnimations: function() { + var target = this._effect._target; + if (!target._activeAnimations) { + target._activeAnimations = []; + } + return target._activeAnimations; + }, + _markTarget: function() { + var animations = this._targetAnimations(); + if (animations.indexOf(this) === -1) { + animations.push(this); + } + }, + _unmarkTarget: function() { + var animations = this._targetAnimations(); + var i = animations.indexOf(this); + if (i !== -1) { + animations.splice(i, 1); + } + }, }; if (WEB_ANIMATIONS_TESTING) { diff --git a/src/keyframe-effect.js b/src/keyframe-effect.js index c79f05d..f6813d8 100644 --- a/src/keyframe-effect.js +++ b/src/keyframe-effect.js @@ -43,6 +43,7 @@ keyframeEffect._hasSameTarget = function(otherTarget) { return target === otherTarget; }; + keyframeEffect._target = target; keyframeEffect._totalDuration = effectTime._totalDuration; keyframeEffect._id = id; return keyframeEffect; diff --git a/src/tick.js b/src/tick.js index 3112f6b..d91ad73 100644 --- a/src/tick.js +++ b/src/tick.js @@ -39,7 +39,7 @@ rafCallbacks = []; if (t < timeline.currentTime) t = timeline.currentTime; - tick(t, true); + timeline._animations = tick(t, true, timeline._animations); processing.forEach(function(entry) { entry[1](t); }); applyPendingEffects(); _now = undefined; @@ -63,7 +63,7 @@ animation._timeline = this; this._animations.push(animation); scope.restart(); - scope.invalidateEffects(); + scope.applyDirtiedAnimation(animation); return animation; } }; @@ -92,8 +92,18 @@ return hasRestartedThisFrame; }; - scope.invalidateEffects = function() { - tick(scope.timeline.currentTime, false); + // This allows us to synchonously apply an animation's effect in case script mutation + // of Animation objects occur between the RAF callback and frame rendering. + scope.applyDirtiedAnimation = function(animation) { + if (inTick) { + return; + } + animation._markTarget(); + var animations = animation._targetAnimations(); + var remainingAnimations = tick(scope.timeline.currentTime, false, animations); + if (remainingAnimations.indexOf(animation) === -1) { + timeline._animations.splice(timeline._animations.indexOf(animation), 1); + } applyPendingEffects(); }; @@ -105,24 +115,28 @@ var t60hz = 1000 / 60; - function tick(t, isAnimationFrame) { + var inTick = false; + function tick(t, isAnimationFrame, updatingAnimations) { + inTick = true; + updatingAnimations.sort(compareAnimations); hasRestartedThisFrame = false; var timeline = scope.timeline; + timeline.currentTime = t; - timeline._animations.sort(compareAnimations); ticking = false; - var updatingAnimations = timeline._animations; - timeline._animations = []; var newPendingClears = []; var newPendingEffects = []; - updatingAnimations = updatingAnimations.filter(function(animation) { + var remainingAnimations = updatingAnimations.filter(function(animation) { animation._tick(t, isAnimationFrame); - if (!animation._inEffect) + if (!animation._inEffect) { newPendingClears.push(animation._effect); - else + animation._unmarkTarget(); + } else { newPendingEffects.push(animation._effect); + animation._markTarget(); + } if (animation._needsTick) ticking = true; @@ -136,10 +150,11 @@ pendingEffects.push.apply(pendingEffects, newPendingClears); pendingEffects.push.apply(pendingEffects, newPendingEffects); - timeline._animations.push.apply(timeline._animations, updatingAnimations); - if (ticking) requestAnimationFrame(function() {}); + + inTick = false; + return remainingAnimations; }; if (WEB_ANIMATIONS_TESTING) { diff --git a/test/js/animation.js b/test/js/animation.js index 714f7ca..b9f554e 100644 --- a/test/js/animation.js +++ b/test/js/animation.js @@ -24,13 +24,13 @@ suite('animation', function() { var a = document.body.animate([], 2000); a.pause(); tick(100); - assert.equal(a.currentTime, 0); + assert.equal(a.currentTime, 0, 'after pause()'); tick(300); a.play(); - assert.equal(a.currentTime, 0); + assert.equal(a.currentTime, 0, 'after play()'); tick(310); - assert.equal(a.currentTime, 0); - assert.equal(a.startTime, 310); + assert.equal(a.currentTime, 0, 'current time after tick'); + assert.equal(a.startTime, 310, 'start time after tick'); var a = document.body.animate([], 2000); a.startTime = -690; @@ -404,8 +404,8 @@ suite('animation', function() { var target = document.createElement('div'); document.body.appendChild(target); tick(0); - var animationBehind = target.animate([{width: '1234px'}, {width: '1234px'}], {duration: 1, fill: 'both'}); - var animationInfront = target.animate([{width: '0px'}, {width: '100px'}], 100); + var animationBehind = target.animate([{width: '1234px'}, {width: '1234px'}], {duration: 1, fill: 'both', id: 'behind'}); + var animationInfront = target.animate([{width: '0px'}, {width: '100px'}], {duration: 100, id: 'infront'}); assert.equal(getComputedStyle(target).width, '0px'); animationInfront.currentTime = 50; assert.equal(getComputedStyle(target).width, '50px'); From e5a1e9960d0d294e2aa15f3849bb0cfbe940dd8a Mon Sep 17 00:00:00 2001 From: Alan Cutter Date: Tue, 2 Aug 2016 15:11:26 +1000 Subject: [PATCH 2/5] Yay memory management --- src/tick.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tick.js b/src/tick.js index d91ad73..5792bd9 100644 --- a/src/tick.js +++ b/src/tick.js @@ -39,6 +39,7 @@ rafCallbacks = []; if (t < timeline.currentTime) t = timeline.currentTime; + timeline._animations.sort(compareAnimations); timeline._animations = tick(t, true, timeline._animations); processing.forEach(function(entry) { entry[1](t); }); applyPendingEffects(); @@ -100,7 +101,8 @@ } animation._markTarget(); var animations = animation._targetAnimations(); - var remainingAnimations = tick(scope.timeline.currentTime, false, animations); + animations.sort(compareAnimations); + var remainingAnimations = tick(scope.timeline.currentTime, false, animations.slice()); if (remainingAnimations.indexOf(animation) === -1) { timeline._animations.splice(timeline._animations.indexOf(animation), 1); } @@ -118,7 +120,6 @@ var inTick = false; function tick(t, isAnimationFrame, updatingAnimations) { inTick = true; - updatingAnimations.sort(compareAnimations); hasRestartedThisFrame = false; var timeline = scope.timeline; From 6ff0b69ba758138f5a8f36733bbdcedac09e8e4f Mon Sep 17 00:00:00 2001 From: Alan Cutter Date: Tue, 2 Aug 2016 16:57:48 +1000 Subject: [PATCH 3/5] Make tests pass --- Gruntfile.js | 2 +- src/animation.js | 1 + src/tick.js | 31 +++++++++++++++++++++---------- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index 62a2c69..aeeb406 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -237,7 +237,7 @@ module.exports = function(grunt) { configCallback(karmaConfig); karmaConfig.client.testharnessTests = require('./test/web-platform-tests-expectations.js'); karmaConfig.client.testharnessTests.testURLList = testFiles; - karmaConfig.proxies['/base/polyfill.js'] = '/base/' + task.target + '.dev.js'; + karmaConfig.proxies['/base/polyfill.js'] = '/base/' + task.target + '.min.js'; karmaConfig.files.push('test/karma-testharness-adapter.js'); var servedFiles = [ 'test/web-platform-tests/resources/**', diff --git a/src/animation.js b/src/animation.js index 58e3918..2bd9229 100644 --- a/src/animation.js +++ b/src/animation.js @@ -191,6 +191,7 @@ this._inEffect = false; this._idle = true; this._paused = false; + this._isFinished = true; this._finishedFlag = true; this._currentTime = 0; this._startTime = null; diff --git a/src/tick.js b/src/tick.js index 5792bd9..08b0a14 100644 --- a/src/tick.js +++ b/src/tick.js @@ -40,7 +40,7 @@ if (t < timeline.currentTime) t = timeline.currentTime; timeline._animations.sort(compareAnimations); - timeline._animations = tick(t, true, timeline._animations); + timeline._animations = tick(t, true, timeline._animations)[0]; processing.forEach(function(entry) { entry[1](t); }); applyPendingEffects(); _now = undefined; @@ -93,8 +93,10 @@ return hasRestartedThisFrame; }; - // This allows us to synchonously apply an animation's effect in case script mutation - // of Animation objects occur between the RAF callback and frame rendering. + // RAF is supposed to be the last script to occur before frame rendering but not + // all browsers behave like this. This function is for synchonously updating an + // animation's effects whenever its state is mutated by script to work around + // incorrect script execution ordering by the browser. scope.applyDirtiedAnimation = function(animation) { if (inTick) { return; @@ -102,10 +104,13 @@ animation._markTarget(); var animations = animation._targetAnimations(); animations.sort(compareAnimations); - var remainingAnimations = tick(scope.timeline.currentTime, false, animations.slice()); - if (remainingAnimations.indexOf(animation) === -1) { - timeline._animations.splice(timeline._animations.indexOf(animation), 1); - } + var inactiveAnimations = tick(scope.timeline.currentTime, false, animations.slice())[1]; + inactiveAnimations.forEach(function(animation) { + var index = timeline._animations.indexOf(animation); + if (index !== -1) { + timeline._animations.splice(index, 1); + } + }); applyPendingEffects(); }; @@ -128,7 +133,9 @@ var newPendingClears = []; var newPendingEffects = []; - var remainingAnimations = updatingAnimations.filter(function(animation) { + var activeAnimations = []; + var inactiveAnimations = []; + updatingAnimations.forEach(function(animation) { animation._tick(t, isAnimationFrame); if (!animation._inEffect) { @@ -144,7 +151,11 @@ var alive = animation._inEffect || animation._needsTick; animation._inTimeline = alive; - return alive; + if (alive) { + activeAnimations.push(animation); + } else { + inactiveAnimations.push(animation); + } }); // FIXME: Should remove dupliactes from pendingEffects. @@ -155,7 +166,7 @@ requestAnimationFrame(function() {}); inTick = false; - return remainingAnimations; + return [activeAnimations, inactiveAnimations]; }; if (WEB_ANIMATIONS_TESTING) { From 242a217e0e1b008295fe89e8c7af64fd1e788e2e Mon Sep 17 00:00:00 2001 From: Alan Cutter Date: Tue, 2 Aug 2016 19:46:51 +1000 Subject: [PATCH 4/5] Don't change tests --- test/js/animation.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/js/animation.js b/test/js/animation.js index b9f554e..714f7ca 100644 --- a/test/js/animation.js +++ b/test/js/animation.js @@ -24,13 +24,13 @@ suite('animation', function() { var a = document.body.animate([], 2000); a.pause(); tick(100); - assert.equal(a.currentTime, 0, 'after pause()'); + assert.equal(a.currentTime, 0); tick(300); a.play(); - assert.equal(a.currentTime, 0, 'after play()'); + assert.equal(a.currentTime, 0); tick(310); - assert.equal(a.currentTime, 0, 'current time after tick'); - assert.equal(a.startTime, 310, 'start time after tick'); + assert.equal(a.currentTime, 0); + assert.equal(a.startTime, 310); var a = document.body.animate([], 2000); a.startTime = -690; @@ -404,8 +404,8 @@ suite('animation', function() { var target = document.createElement('div'); document.body.appendChild(target); tick(0); - var animationBehind = target.animate([{width: '1234px'}, {width: '1234px'}], {duration: 1, fill: 'both', id: 'behind'}); - var animationInfront = target.animate([{width: '0px'}, {width: '100px'}], {duration: 100, id: 'infront'}); + var animationBehind = target.animate([{width: '1234px'}, {width: '1234px'}], {duration: 1, fill: 'both'}); + var animationInfront = target.animate([{width: '0px'}, {width: '100px'}], 100); assert.equal(getComputedStyle(target).width, '0px'); animationInfront.currentTime = 50; assert.equal(getComputedStyle(target).width, '50px'); From 24f7a24f6fc5f27f879c386aa78d369d4f0bb30b Mon Sep 17 00:00:00 2001 From: Alan Cutter Date: Tue, 2 Aug 2016 19:48:20 +1000 Subject: [PATCH 5/5] lint --- src/animation.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/animation.js b/src/animation.js index 2bd9229..e9f6482 100644 --- a/src/animation.js +++ b/src/animation.js @@ -265,9 +265,9 @@ }, _unmarkTarget: function() { var animations = this._targetAnimations(); - var i = animations.indexOf(this); - if (i !== -1) { - animations.splice(i, 1); + var index = animations.indexOf(this); + if (index !== -1) { + animations.splice(index, 1); } }, };