From 7c65a664bb4f3ab6bdbb9604414d01debcca6847 Mon Sep 17 00:00:00 2001 From: Saivan Date: Tue, 29 May 2018 18:28:15 +1000 Subject: [PATCH] Diagnosing the declarative animations and suggesting changes --- dirty.html | 19 ++++++------ src/controller.js | 2 ++ src/runner.js | 29 +++++++++--------- src/timeline.js | 76 +++++++++++++++++++++++++++-------------------- 4 files changed, 68 insertions(+), 58 deletions(-) diff --git a/dirty.html b/dirty.html index 21cf58e..bb91400 100644 --- a/dirty.html +++ b/dirty.html @@ -74,8 +74,8 @@ function getColor(t) { // } // } -var bla = SVG('').size(0, 0).move(200, 200).addTo('svg') -bla.animate().size(220, 200).queue(null, console.log) +// var bla = SVG('').size(0, 0).move(200, 200).addTo('svg') +// bla.animate().size(220, 200).queue(null, console.log) // var randPoint = (x = 50, y = 50) => [ // Math.random() * 100 - 50 + x, @@ -101,14 +101,13 @@ bla.animate().size(220, 200).queue(null, console.log) // ]) // }) -// var mover = SVG('').size(50, 50).center(100, 100).addTo('svg') -// var anim = mover.animate(new SVG.Spring(500, 10)).move(500, 500) -// -// SVG.on(document, 'mousemove', function (e) { -// //mover.animate(SVG.PID()).move(e.pageX, e.pageY) -// var p = mover.point(e.pageX, e.pageY) -// anim.center(p.x, p.y) -// }) +var mover = SVG('').size(50, 50).center(100, 100).addTo('svg') +var anim = mover.animate(new SVG.Spring(500, 10)).move(500, 500) + +SVG.on(document, 'mousemove', function (e) { + var p = mover.point(e.pageX, e.pageY) + anim.center(p.x, p.y) +}) // var timeline = new SVG.Timeline().pause() // var runner = new SVG.Runner(100000) diff --git a/src/controller.js b/src/controller.js index 359dc9f..b2c7d13 100644 --- a/src/controller.js +++ b/src/controller.js @@ -120,6 +120,8 @@ SVG.Spring = SVG.invent ({ extend: { step: function (current, target, dt, c) { +console.log(current, target, dt); + c.done = dt == Infinity if(dt == Infinity) return target if(dt == 0) return current diff --git a/src/runner.js b/src/runner.js index 3b658c2..5edf44a 100644 --- a/src/runner.js +++ b/src/runner.js @@ -150,12 +150,16 @@ SVG.Runner = SVG.invent({ // FIXME: timeline is already set when used in normal ways // but for manual runners we need that here anyway // so just have doubled code? + // TODO: Nope, thats not good - lets talk about this :P timeline.schedule(this, delay, when) this.timeline(timeline) return this }, - // schedule a runner + // FIXME: These functions shouldn't exist, and if they haaaaaaave to, + // then they should be private at the very least + // Why do we need them? Cant you just integrate it into loop then call + // loop from the constructor directly? init: function (duration, delay, when) { // Initialise the default parameters var times = 0 @@ -180,6 +184,7 @@ SVG.Runner = SVG.invent({ return this.schedule(this.timeline(), delay, when) }, + // FIXME: See above initLoop: function (duration, times, swing) { // If we have an object, unpack the values if (typeof duration == 'object') { @@ -195,6 +200,8 @@ SVG.Runner = SVG.invent({ return this.init(duration) }, + // FIXME: This definitely shouldn't make a new runner, this should just change + // the looping behaviour of the current runner. loop: function (duration, times, swing) { var runner = new SVG.Runner(duration.duration || duration) if(this._timeline) runner.element(this._timeline) @@ -237,7 +244,6 @@ SVG.Runner = SVG.invent({ return this }, - // Queue a function to run after this runner after (fn) { return this.on('finish', fn, this) }, @@ -250,12 +256,11 @@ SVG.Runner = SVG.invent({ /* Runner animation methods ======================== - Controls how the animation plays + Control how the animation plays */ time: function (time) { if (time == null) return this._time - let dt = time - this._time this.step(dt) return this @@ -274,12 +279,11 @@ SVG.Runner = SVG.invent({ // Work out if we are in range to run the function var timeInside = 0 <= time && time <= duration - var position = time / duration var finished = time >= duration + var position = finished ? 1 : time / duration // If we are on the rising edge, initialise everything, otherwise, // initialise only what needs to be initialised on the rising edge - // var justStarted = this._last <= 0 && time >= 0 var justFinished = this._last <= duration && finished this._initialise() this._last = time @@ -288,11 +292,7 @@ SVG.Runner = SVG.invent({ if(!timeInside && !justFinished) return finished // Run the runner and store the last time it was run - var runnersFinished = this._run( - this._isDeclarative ? dt - : finished ? 1 - : position - ) + var runnersFinished = this._run(this._isDeclarative ? dt : position) finished = (this._isDeclarative && runnersFinished) || (!this._isDeclarative && finished) @@ -349,7 +349,6 @@ SVG.Runner = SVG.invent({ for(var i = name.length; i--;) { delete this.tags[name[i]] } - return this }, @@ -396,8 +395,8 @@ SVG.Runner = SVG.invent({ } }, - // Run each run function for the position given - _run: function (position) { + // Run each run function for the position or dt given + _run: function (positionOrDt) { // Run all of the _queue directly var allfinished = true @@ -409,7 +408,7 @@ SVG.Runner = SVG.invent({ // Run the function if its not finished, we keep track of the finished // flag for the sake of declarative _queue current.finished = current.finished - || (current.runner.call(this, position) === true) + || (current.runner.call(this, positionOrDt) === true) allfinished = allfinished && current.finished } diff --git a/src/timeline.js b/src/timeline.js index 35bc71c..f9f3b5b 100644 --- a/src/timeline.js +++ b/src/timeline.js @@ -7,6 +7,8 @@ SVG.easing = { '<': function (pos) { return -Math.cos(pos * Math.PI / 2) + 1 } } +var time = performance || Date + SVG.Timeline = SVG.invent({ // Construct a new timeline on the given element @@ -14,6 +16,9 @@ SVG.Timeline = SVG.invent({ // Store a reference to the element to call its parent methods this._element = element || null + this._timeSource = function () { + return time.now() + } // Store the timing variables this._startTime = 0 @@ -29,7 +34,8 @@ SVG.Timeline = SVG.invent({ this._paused = false this._runners = [] this._time = 0 - this._lastTime = 0 + this._lastSourceTime = 0 + this._lastStepTime = 0 }, extend: { @@ -54,6 +60,10 @@ SVG.Timeline = SVG.invent({ schedule (runner, delay, when) { + // TODO: If no runner is provided, get the whole schedule + // TODO: If a runner is provided with no delay or when, get its + // starting time and delay + runner.unschedule() // The start time for the next animation can either be given explicitly, @@ -97,8 +107,10 @@ SVG.Timeline = SVG.invent({ return this }, - // FIXME: this does not work. Setting the nextFrame to null alone is not working - // We need to remove our frames from the animator somehow + // FIXME: this does not work. Setting the nextFrame to null alone is not + // working we need to remove our frames from the animator somehow + // TODO: This method shouldn't exist... it isn't required. Setting pause + // to true is suffcient. The user shouldn't controll the animation frames cancel () { // SVG.Animator.cancel(this._nextFrame) this._nextFrame = null @@ -128,56 +140,55 @@ SVG.Timeline = SVG.invent({ return this }, - // FIXME: rewrite this to use the speed method reverse (yes) { - this._speed = Math.abs(this._speed) * yes ? -1 : 1 + var currentSpeed = this.speed() + this.speed(-currentSpeed) return this }, seek (dt) { - // what to do here? - // we cannot just set a new time - // also calling step does not result in anything - // because step is getting called with the current real time which - // will reset it to the old flow - - // only way is to change lastTime to the current time + what we want - this._lastTime -= dt + this._time += dt return this }, - time (t) { - if(t == null) return this._time - this._time = t + time (newTime) { + if(newTime == null) return this._time + this._time = newTime return this }, persist (dtOrForever) { - if(tdOrForever == null) return this._persist - + if (tdOrForever == null) return this._persist this._persist = dtOrForever return this }, + source (fn) { + if (fn == null) return this._timeSource + this._timeSource = fn + return this + }, + _step (time) { + // FIXME: User should be able to step manually - // move this check to the very bottom - // or mixup the continue, step logic - // If we are paused, just exit + // FIXME: No they shouldn't. _step is a hidden function and should + // remain hidden because it is intended to be called by + // requestAnimationFrame only. If they want to manually step, + // they can just call seek a bunch of times with a _timeSource that + // always returns 0. if (this._paused) return // Get the time delta from the last time and update the time // TODO: Deal with window.blur window.focus to pause animations - // HACK: We keep the time below 50ms to avoid driving animations crazy - // FIXME: We cannot seek to -time because speed fucks this up - var dt = this._speed * ((time - this._lastTime) || 16) + var time = this._timeSource() + this._lastSourceTime = time + var dtSource = ((time - this._lastSourceTime) || 16) + var dtTime = this._speed * dtSource + (this._time - this._lastStepTime) - // we cannot do that. Doesnt work when user wants to manually step (or seek) - dt = dt < 50 ? dt : 16 // If we missed alot of time, ignore - this._lastTime = time - - // FIXME: this is not used - this._time += dt + // Update the time + this._time += dtTime + this._lastStepTime = this._time // Run all of the runners directly var runnersLeft = false @@ -188,7 +199,7 @@ SVG.Timeline = SVG.invent({ // If this runner is still going, signal that we need another animation // frame, otherwise, remove the completed runner - var finished = runner.step(dt).done + var finished = runner.step(dtTime).done if (!finished) { runnersLeft = true } else if(this._persist !== true){ @@ -227,8 +238,7 @@ SVG.Timeline = SVG.invent({ // Checks if we are running and continues the animation _continue () { if (this._paused) return this - if (!this._nextFrame) - this._step(this._lastTime) // FIXME: we have to past an absolute time here + if (!this._nextFrame) this._step() return this }, }, -- 2.39.5