Skip to content

Commit

Permalink
- Fx is now frame based rather than time based. This makes things a …
Browse files Browse the repository at this point in the history
…lot easier internally, and makes sure that even pause / resume operations are completely in sync.

 - the frames are calculated with the duration passed as an option, so it works as usual.
 - added a "frames" option, so that Fx can now work without a pre-determined duration.
 - rearranged some internal methods and properties.
 - added a "stop" method, that basically pauses the animation and fires the stop event.
 - added the "isRunning" method.
 - every method and option works as before.
  • Loading branch information
kamicane authored and cpojer committed Oct 15, 2010
1 parent 7ea65fb commit 6b4feb3
Showing 1 changed file with 44 additions and 63 deletions.
107 changes: 44 additions & 63 deletions Source/Fx/Fx.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ var Fx = this.Fx = new Class({
onCancel: nil,
onComplete: nil,
*/
fps: 50,
fps: 60,
unit: false,
duration: 500,
frames: null,
link: 'ignore'
},

Expand All @@ -43,17 +44,12 @@ var Fx = this.Fx = new Class({
};
},

step: function(time){
if (!this.time || this.stopped){
this.time = time - this.time;
this.stopped = false;
}
if (time < this.time + this.options.duration){
var delta = this.transition((time - this.time) / this.options.duration);
step: function(){
if (this.frames > this.frame){
var delta = this.transition(++this.frame / this.frames);
this.set(this.compute(this.from, this.to, delta));
} else {
this.set(this.compute(this.from, this.to, 1));
this.complete();
this.stop();
}
},

Expand All @@ -66,7 +62,7 @@ var Fx = this.Fx = new Class({
},

check: function(){
if (!this.timer) return true;
if (!this.isRunning()) return true;
switch (this.options.link){
case 'cancel': this.cancel(); return true;
case 'chain': this.chain(this.caller.pass(arguments, this)); return false;
Expand All @@ -76,66 +72,51 @@ var Fx = this.Fx = new Class({

start: function(from, to){
if (!this.check(from, to)) return this;
var duration = this.options.duration;
this.options.duration = Fx.Durations[duration] || duration.toInt();
this.from = from;
this.to = to;
this.time = 0;
this.completed = this.stopped = false;
this.frame = -1;
this.transition = this.getTransition();
this.startTimer();
this.onStart();
var frames = this.options.frames, fps = this.options.fps, duration = this.options.duration;
this.frames = frames || Math.round((Fx.Durations[duration] || duration.toInt()) / (1000 / fps));
pushInstance.call(this, fps);
return this;
},

complete: function(){
if (this.stopTimer()){
this.onComplete();
this.completed = true;

stop: function(){
if (this.isRunning){
pullInstance.call(this, this.options.fps);
if (this.frames == this.frame){
this.fireEvent('complete', this.subject);
if (!this.callChain()) this.fireEvent('chainComplete', this.subject);
} else {
this.fireEvent('stop', this.subject);
}
}
return this;
},

cancel: function(){
if (this.stopTimer()) this.onCancel();
if (this.isRunning()){
pullInstance.call(this, this.options.fps);
this.frame = this.frames;
this.fireEvent('cancel', this.subject).clearChain();
}
return this;
},

onStart: function(){
this.fireEvent('start', this.subject);
},

onComplete: function(){
this.fireEvent('complete', this.subject);
if (!this.callChain()) this.fireEvent('chainComplete', this.subject);
},

onCancel: function(){
this.fireEvent('cancel', this.subject).clearChain();
},


pause: function(){
this.stopTimer();
if (this.isRunning()) pullInstance.call(this, this.options.fps);
return this;
},

resume: function(){
if (!this.completed) this.startTimer();
if ((this.frames > this.frame) && !this.isRunning()) pushInstance.call(this, this.options.fps);
return this;
},

stopTimer: function(){
if (!this.timer) return false;
this.time = Date.now() - this.time;
this.stopped = true;
this.timer = removeInstance(this, this.options.fps);
return true;
},

startTimer: function(){
if (this.timer) return false;
this.timer = addInstance(this, this.options.fps);
return true;

isRunning: function(){
var list = instances[this.options.fps];
return list && list.contains(this);
}

});
Expand All @@ -151,26 +132,26 @@ Fx.Durations = {'short': 250, 'normal': 500, 'long': 1000};
var instances = {}, timers = {};

var loop = function(){
var time = Date.now();
for (var i = this.length; i--;){
if (this[i]) this[i].step(time);
if ((instance = this[i])) instance.step();

This comment has been minimized.

Copy link
@appden

appden Oct 17, 2010

Member

Accidental global!

}
};

var addInstance = function(instance, fps){
var pushInstance = function(fps){
var list = instances[fps] || (instances[fps] = []);
list.push(instance);
list.push(this);
if (!timers[fps]) timers[fps] = loop.periodical(Math.round(1000 / fps), list);
return true;
};

var removeInstance = function(instance, fps){
var pullInstance = function(fps){
var list = instances[fps];
if (list){
list.erase(instance);
if (!list.length && timers[fps]) timers[fps] = clearInterval(timers[fps]);
list.erase(this);
if (!list.length && timers[fps]){
delete instances[fps];
timers[fps] = clearInterval(timers[fps]);
}
}
return false;
};

})();

13 comments on commit 6b4feb3

@sebmarkbage
Copy link
Member

Choose a reason for hiding this comment

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

I don't think I like this. The benefit of time based animations is that they can catch up if the animation runs slow. It also goes against the future best practice of only updating the DOM during "beforepaint" events. Allowing frames to be ignored as required.

@appden
Copy link
Member

@appden appden commented on 6b4feb3 Oct 17, 2010

Choose a reason for hiding this comment

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

I also don't like this. If you tell the animation to run for 600ms, you would have 36 frames, and you're relying heavily on each frame executing at an average of one every 16.67 ms. In actuality, you might find those frames run an average of every 18ms, which would cause your animation to take 648ms long! For meticulously timed animations, this could be very bad!

@kamicane
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, however this way, even on slower computers, the animations will be fully visible without jumping. Also, with frames, it is much easier to handle perfect syncing for start / pause / resume.

@sebmarkbage
Copy link
Member

Choose a reason for hiding this comment

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

You could still handle perfect syncing by treating time as a global frame counter.

time = Math.floor(Date.now() / fps);

Then you're sure to have perfect frame integers, and it still allows skipping of frames, if necessary.

@smoofles
Copy link

Choose a reason for hiding this comment

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

Regarding slower computers (I am assuming netbooks, tablets, etc?): with the browsers moving to hardware accelerated animation & layout, is it still neccessary to do the gruntwork like this? Personally, I'd rather have the animations in a UI done in exactly the time I specified, than having users wait for the animation to (smoothly) finish. If they're running on a slow computer, choppy animations is something they will have to deal with/expect. My thinking is the users would rather have that than unnecessary wait times.

However, I do see the other side of the argument, too. Maybe it's something that could be optional, so neither group (time vs frame based animators) has to deal with results they didn't intend?

(All that assuming I am reading the commit correctly and haven't missed something dealing with my concerns already…)

@eerne
Copy link
Contributor

@eerne eerne commented on 6b4feb3 Oct 17, 2010

Choose a reason for hiding this comment

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

++ for option.

Sometimes it's really important that it's accurat, but in other situation it's more important that its smooth and never jumps.

@digitarald
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would this be in 1.3.1?

Just dropped this into my apps and some animations run noticeable slow in VM IE. From a user experience, time matters and not the speed of the system. An element should appear in specific time period, no matter which system the user is on … I don't think of many use cases where I would use frames over duration.

@ibolmo
Copy link
Member

@ibolmo ibolmo commented on 6b4feb3 Oct 18, 2010

Choose a reason for hiding this comment

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

I like the universal time that calyptus mentioned. Seems like the middle of the road option.

@digitarald
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, after some more checking: Several animations run ridiculously slow IE8 in VM, mostly alpha blending for elements or modal dialog background.

@seanmonstar
Copy link

Choose a reason for hiding this comment

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

agreed with harald. i dont want to have watch a painfullly slow animation, even if it is complete and I see all frames. I'd rather it skip and be done in a second or half a second or whatever.

@digitarald
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the status on that, will it stay in 1.3.1? I am stuck more and more on overlay fade ins that take painfully long, showing that this is not a compatible change and maybe not a pattern we should prefer over duration.

@SunboX
Copy link

@SunboX SunboX commented on 6b4feb3 Oct 19, 2010

Choose a reason for hiding this comment

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

+1 for time. Time matters!

@ibolmo
Copy link
Member

@ibolmo ibolmo commented on 6b4feb3 Oct 19, 2010

Choose a reason for hiding this comment

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

Actually this needs to be reverted since it's not a bug fix. This should go in 1.4wip, no?

Please sign in to comment.