-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Implement a basic animation loop. #57
Conversation
The animation loop has the following features: + Decoupled stepping from rendering so that stepping can be done using a constant time step which is very important in any numerical integration. + API that allows passing in pure functions. + Pauses when browser tab is not visible (more accurately when the ticker is called too infrequently). + Stops automatically when there are no more animations to run. + Supports rendering with interpolation.
animationLoop.accumulatedTime = 0; | ||
} | ||
|
||
while (animationLoop.accumulatedTime > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it be done in the non-blocking way? With enough of elements this one will freeze the ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, no. The loop function is called by requestAnimationFrame
. Everything inside the loop needs to run in one frame. I agree that with enough elements the UI will freeze. But if you get to that point you should probably dial down on animations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you do it any other way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple independent loops will still run in sync. requestAnimationFrame
calls all listeners in the same tick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also one more concern. If any of steps fails for any reason, whole animation process will fail.
@@ -52,6 +59,42 @@ function mergeDiffObj(a, b, onRemove) { | |||
return ret; | |||
} | |||
|
|||
// TODO: refactor common logic with updateCurrVals and updateCurrV | |||
function interpolateVals(alpha, nextVals, prevVals) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can go to utils or something
Woah 👍 |
what about that memory usage? |
What's the bottleneck on memory here? We allocate quite a bit in some places (ahem, |
Believe it or not the reason it is using so much less memory in the before example is because most of the 1000 elements are off screen. When they are all on screen it matches the usage from the after example. I can reliably reproduce this. |
ah, true |
} | ||
|
||
// Render and filter in one iteration. | ||
reverseFilter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the perf diff of the reverseFilter optimization vs. inlining renderSubscriber?
const alpha = 1 + animationLoop.accumulatedTime / timeStep;
animationLoop.state = animationLoop.state.filter(subscriber => {
subscriber.render(alpha, subscriber.value, subscriber.prevValue);
return subscriber.active;
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look: http://jsperf.com/function-inside-raf.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dam, nice one!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks for that!
@nkbt what do you think? It solves #68 (comment) so right off the bat that's pretty good. I'll give this a better read tonight/tomorrow |
This introduces inaccuracies intentionally, correct? The most accurate thing to do would be to recover by advancing two steps in the next frame. But what you're saying is that it is more visually appealing to recover from a dropped frame by interpolating to some value in between the previously displayed value, and the most accurate value? Is my understanding of your approach correct? |
I don’t think you understand correctly. The simulation is recovering by advancing 2 steps, but the fact that there are frames in which a different number of steps are taken (usually 0, 1 or 2) means that the animation will not look good. I will try to provide an example of what happens somewhere in the middle of an animation with the time step of the simulation set to
Without interpolation at Frame 2 the objects in our simulation would not move at all and at Frame 3 they would move 2 steps. This is what would cause the jitter. Hope this makes any sense. |
@chenglou so far I do not have any major concerns. I want to give it a try on my current project to see how it works in real app. I will comment in the next hour or two. |
I checked on my project, with some animations in there. Not too many though. Can't see any difference, which is a good sign. I also reviewed more the source code in Webstorm. It is kind of opinionated, but it is quite difficult to work with this code - didn't see that much OOP in JS for months. I would definitely consider refactoring the animationLoop into bunch of pure functions. |
@nkbt I am totally open for refactoring and am all for pure functions but I fail to see what I could simplify. Can you give me some pointers? |
@iclanzan well it is kind of full-refactoring of the code, removing If we decide to go functional, then you will need to rebase against the latest master first and then we can work on the code itself. |
Dealing with animations is by nature stateful and with side effects. That state needs to live somewhere. If state stays in the animation loop then you can have pure |
That is right, I would just prefer to keep state in closure and not in the object. |
Oh, that is easily doable. Something like this https://github.com/iclanzan/momentum/blob/master/lib/animationLoop.js? Ignore the emitter. |
Alright. I will write some tests for what we have right now so this can be merged because there are a number of people waiting for the fixes that the animation loop brings. After that we can play with refactoring and benchmarking. Does that sound good? |
I’m curious how @DrBoolean would approach the animation loop from a functional perspective. |
I've been trying out this branch with a carousel i'm building. The improvements make iOS Safari go from being unusable to performing very fluidly. |
@AndrewIngram That is terrific news! |
Hm, there does seem to be a noticeable lag between interactions and the animation though. Which isn't present in the iOS Simulator, just on actual devices. |
Any way you could record the screen? And by interactions do you mean when you move the finger across the screen? |
Recording the screen might be tricky, i'll try and figure something out. But essentially the endValue of the spring is set as the result of a touchMove event. So for slow movement i'd expect the position to stay more-or-less in sync, and to drag slightly behind for fast movements. This is how it behave on desktop browsers and in the iOS simulator. On an actual device there's a delay of about half a second before spring suddenly jumps into action, then there appears to be a relatively consistent lag behind all subsequent touchMove events. |
If the animations are running smoothly when you don’t interact with the device then it could be a problem with how touch events are handled. I know in the demos they trigger unnecessary re-renders which cause lag. This is made worse by the fact that these events can fire more frequent than can be rendered. I tested demo 1 on the iPhone (real device) and it works smoothly up to 100 elements when I’m not touching the screen. Touching the screen causes some jitter. |
Yeah I didn't want to comment on the OOP-ness because it's the idea that's important here. But I did mind a bit that it's OO. Regarding state, this library should enable time-travel and such (@gaearon) and I really don't want any state that lives in some place that's gonna be hard to reach. Closures are fine and being inside components is totally fine. Also is it just me or do the instabilities not happen unless I switch away browser tab? 1k chat heads on master. @AndrewIngram just to make sure: the touch delay issue happens on this PR in particular and not on master? |
@chenglou They happen without switching tabs. You do have to wait a bit for more of the elements to start animating since they all start in the top left corner. Just move your mouse around the screen and you should see it get unstable. I can always reproduce it. |
@chenglou pretty sure it happens on master too, but it's hard to be sure because the poor performance means it's jumping around a lot |
Great, nice to hear it's not this PR's problem =). If you want, please file an issue and a small repro (like you've written here) for it |
Super cool stuff @iclanzan, great read! |
Just checking in again. The poor performance I was experiencing was due to some poorly written components on my end within the Spring. This pull requests still performs better than master, and I still have concerns about mobile performance, but the situation isn't quite as bad as I thought. |
Yeah I'm ready to merge this. The OOP-ness doesn't matter. Not user-facing so we'll gradually work on it. |
@chenglou I guess you can merge it and we will keep improving it. |
Should we squash this? Also, would be nice to be merged without conflict |
Here you go :) |
Implement a basic animation loop.
BOOM! Fixes #31 |
Some changes include #57, demo 4 photo gallery clarification, actual production build (speed speed speed), etc.
I have adapted this from a project I started working on a few days ago.
The animation loop has the following features:
a constant time step which is very important in any numerical
integration.
ticker is called too infrequently).
I would really appreciate some feedback to make sure we take this in the right direction.
API
Explanation
This library is basically a very simplified physics simulation engine for React. Because of this we can learn from what others have done in the gaming industry.
I have done quite a bit of reading on the subject and looked under the hood of a number of game engines before writing this animation loop. Here are some good articles on the subject.
So what is the animation loop? The animation loop is the heartbeat of our engine and is responsible for calling the function that updates our physic simulation whenever a frame can be rendered. One update cycle is called a step and in the case of browsers requestAnimationFrame dictates when a frame can be rendered.
At any given step, a physics simulation, just like this library, uses numerical integrations to compute the state of the “world” from the previous step’s state. As such, this state is only an approximation of how a similar system would behave in the real world.
In order to ensure the stability of the system and to have accurate integrations it is important to do the stepping in fixed time increments or fixed time step. The problem with
requestAnimationFrame
is that it gets called at irregular (many times dramatic) intervals.Step
In order to solve this problem stepping needs to be decoupled from rendering. The way this works is whenever
rAF
is called we accumulate time. When enough time is accumulated we advance our simulation. If for example at any point in time we have accumulated35ms
and our chosen time step is16.6ms
then the system will advance by 2 steps in one frame.Render
When it’s time to render, we inevitably run into an awkward situation where our world stays the same for 2 frames in a row when not enough time accumulates to make a step. Many times this is followed by a frame in which two steps are made. This leads to an irritating jitter when rendered. The solution is straightforward though. Knowing the current and previous state as well as the unused accumulated time, we can interpolate all values resulting in buttery smooth animations.
Time Step
The time step has a direct impact on the progression of the simulation. The lower the time step, the more accurate and stable the simulation gets. The higher the time step, the more brittle it becomes. In my experience the system becomes very unstable at any time step above
1/20
seconds or0.05
ms with objects flying everywhere and does not noticeably improve accuracy for time steps below1/120
seconds or0.008
ms. Remember changing the time step does not mean a change in the rendered FPS. It just means the simulation does more or less steps per frame. What it does mean however, is that it impacts performance. A higher time step means more performance at the expense of stability and accuracy. I think a safe bet is a time step of1/60
.