Simple animation method with requestAnimationFrame code structure

Question

I'm writing a animate method to perform DOM animations. I know there are a lot of libraries out there that already does that (and I use some ideas of them) but I usually prefer writing my own code to pinpoint my problem.

The code below is working perfectly, but something with the structure is making me uncomfortable. I think it is because I use references to itself just before the objects dies, so I'm not sure if I'm reusing the animation object or creating new ones. Apparently there is no memory leak.

Can someone give it a look? Fell free to suggest code modifications even if they are unrelated with the memory leak.

window.animate = function(options) {

    var fps = options.fps || 30;
    var x = options.x || null;
    var y = options.y || null;
    var duration = Math.ceil((options.duration / 1000) * fps);
    var onProgress = options.onProgress || null;
    var onComplete = options.onComplete || null;        
    var easing = easing || 'easeInOutQuad';

    var animation = {
        fps: fps,
        x: x,
        y: y,
        duration: duration,
        onProgress: onProgress,
        onComplete: onComplete,
        easing: easing,
        _id: null,
        _iteration: 1,
        _lastFrameOccurrence: 0,
        _draw: function() {

            var $this = this;

            var now = Date.now();
            var delta = now - $this._lastFrameOccurrence;
            var interval = 1000 / $this.fps;

            if (delta > interval) {

                $this._lastFrameOccurrence = now - (delta % interval);

                var response = {};
                var factor = window.w.easing[$this.easing]($this._iteration, $this.duration);
                for (var i in $this.x)
                    if ($this.x.hasOwnProperty(i))
                        response[i] = $this.x[i] + ($this.y[i] - $this.x[i]) * factor;

                if ($this.onProgress instanceof Function)
                    $this.onProgress(response);

                $this._iteration++;

                if ($this._iteration > $this.duration)
                    if ($this.onComplete instanceof Function)
                        $this.onComplete();

            };

            if ($this._iteration <= $this.duration) {
                $this._id = requestAnimationFrame(function() {
                    $this._draw();
                });
            } else {
                cancelAnimationFrame($this._id);
            };

        }
    };

    animation._draw();

};

                w.animate({
                    x: { opacity: 1 },
                    y: { opacity: 0 },
                    duration: 500,
                    onProgress: function(r) { console.log(r); },
                    onComplete: function() {}
                });

Answer 1

Firstly, the usage example at the end doesn't really make sense. You're passing 5 arguments to the function, but it seems to only take 1 options object.

Structurally, it seems overwrought to create the animation object. Everything can be handled with closures, as far as I can tell. Somewhat in the same vein, I see no reason for the $ and _ prefixes on half the stuff.

Speaking of naming, the x and y options should probably be called from and to or something. It took me a few moments to realize that's what they are - not coordinates or something similar.

Your setup also seems conflicted, or at any rate complicated. You're using frame count, FPS, and duration (time) to track things, though these may contradict each other. You can't really dictate FPS; FPS is a measured value. If the browser is busy, FPS will drop, meaning each iteration will take longer. But you're calculating animation progression based on _iteration versus duration (which has since been redefined as the target number of frames, not time).

So I can say "I want 600,000 FPS!" but I won't get that kind of performance (requestAnimationFrame aims for 30 to 60'ish FPS, usually). But I can also say I want a duration (time-wise) of 1 second, yet the animation will still run 600,000 iterations. However this will not complete in 1 second.

It makes slightly more sense if you want low FPS, since your function just skips some onProgress calls. But low FPS is usually not considered a thing to strive for, really. In case you do want it, it might be easier to use setTimeout instead of requestAnimationFrame and ignoring updates.

Typically, animations have a set duration (in time), and FPS is whatever it is. You can't control it anyway. And so counting frames usually isn't very useful. Instead you do:

animationProgress = (now - startTime) / totalDuration

And you clamp that to 0...1. That's the input to the easing function.

Lastly, there's no need to call cancelAnimationFrame unless you've already called requestAnimationFrame and want to undo that before the next repaint.

Here's a pretty basic implementation:

function animate(options) {
  options = options || {};
  
  // defaults
  var duration = options.duration || 1000,
      ease = options.easing || function (a) { return a }, // basic linear easing
      onProgress = options.onProgress || function () {},
      onComplete = options.onComplete || function () {},
      from = options.from || {},
      to = options.to || {};
  
  // runtime variables
  var startTime = Date.now();
  
  function update() {
    var deltaTime = Date.now() - startTime,
        progress = Math.min(deltaTime / duration, 1),
        factor = ease(progress),
        values = {},
        property;
    
    for(property in from) {
      if(from.hasOwnProperty(property) && to.hasOwnProperty(property)) {
        values[property] = from[property] + (to[property] - from[property]) * factor;
      }
    }
    
    onProgress(values);
    
    if(progress === 1) {
      onComplete(deltaTime);
    } else {
      requestAnimationFrame(update);
    }
  }
  
  requestAnimationFrame(update);
}

// ==============================

// Example
animate({
  from: { a: 0 },
  to:   { a: 100 },
  duration: 500,
  onProgress: function (values) {
    document.getElementsByTagName("pre")[0].innerHTML += values.a + "\n";
  },
  onComplete: function (actualDuration, averageFps) {
    document.getElementsByTagName("pre")[0].innerHTML += "Done in " + actualDuration + "ms";
  }
});

<pre></pre>

I've opted to pass the easing function as an option, rather than a name. Thus you can supply your own easing function on the fly, or pass in a predefined function.

Answer 2

window.animate = function(options) {

This would be fine if animate was just... animate. But when you start to add more and more APIs, you might want to house them in a single global instead of putting them all in the global namespace.

var factor = window.w.easing[$this.easing]($this._iteration, $this.duration);

Here's another case. You wouldn't want to have very many globals, unless that's another framework. But I can't really tell, because w doesn't really tell me anything.

---

var fps = options.fps || 30;
var x = options.x || null;
var y = options.y || null;
var duration = Math.ceil((options.duration / 1000) * fps);
var onProgress = options.onProgress || null;
var onComplete = options.onComplete || null;        
var easing = easing || 'easeInOutQuad';

var animation = {

// into

var animation= Object.assign({
  // Your defaults
  fps: 30,
  x: null,
  y: null,
  ...
}, options);

If you can use ES6 APIs, then you can use Object.assign. What it does is merge the object into the first object. If you also happen to use jQuery, lodash or underscore, you can also use their extend methods. jQuery has $.extend which does the same thing.

---

Normally, you would want to reschedule requestAnimationFrame immediately after you call the callback. That way, there is no delay caused by your operations in drawing the next frame. You can cancel it anytime.

draw: function(){
  var id = requestAnimationFrame(this.draw);
  ... do stuff ...
  if(shouldCancel) cancelAnimationFrame(id);
}

---

var now = Date.now();
var delta = now - $this._lastFrameOccurrence;

I believe the Date.now() isn't necessary. requestAnimationFrame callbacks receive a current timestamp as it's first argument.

---

for (var i in $this.x)

I can't tell what $this.x is. But if it's an array, use a regular for loop instead of a for-in.

---

if ($this.onProgress instanceof Function)
    $this.onProgress(response);

$this._iteration++;

if ($this._iteration > $this.duration)
    if ($this.onComplete instanceof Function)
        $this.onComplete();

Instead of checking if it is a function, why not default it to a function that does nothing. That way, you can just call it.

---

animate(
    { left: 100 },
    { left: 300 },
      60,
      function(r) { console.log(r); },
      function() {}
);

I think this is broken. Your animate expects a single object. You have the whole town here.