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

Make draw scheduling more robust. #25

Merged
merged 3 commits into from Dec 2, 2014

Conversation

Projects
None yet
2 participants
@jwmerrill
Contributor

jwmerrill commented Dec 2, 2014

The primary motivation here is to allow the draw scheduler to cooperate with
an external requestAnimationFrame loop, which users can implement via a port,
or using the experimental Monitor package. Unlike fps, this makes it
possible to achieve 60 fps animation in Elm without regularly dropping frames.

In the existing implementation, it is possible to block rendering forever
using an external requestAnimationFrame loop by scheduling a new domUpdate for
the beginning of each animation frame. This cancels the previous draw callback
(using cancelAnimationFrame), and schedules a new draw callback that will not
run until the next frame. If a new domUpdate is requested in the next frame
also, the new draw callback will also be cancelled, and the process can repeat
arbitrarily long until new domUpdates stop being requested.

The new implementation does not use cancelAnimationFrame. Instead, it uses a
small state machine to ensure that only one drawCallback is ever scheduled per
frame.

The new implementation also drops the vendor prefixes on requestAnimationFrame
because no recent browser uses them. In case requestAnimationFrame is not
available, it uses setTimeout(cb, 1000/60). The existing implementation used
an immediate call, but I think it's better to throttle draw calls to make the
behavior more similar to the case where requestAnimationFrame is available.

A final small advantage of the new implementation is that, unlike the existing
implementation, it does not create a new function closure on every draw call.

Make draw scheduling more robust.
The primary motivation here is to allow the draw scheduler to cooperate with
an external requestAnimationFrame loop, which users can implement via a port,
or using the experimental [Monitor][] package. Unlike fps, this makes it
possible to achieve 60 fps animation in Elm without regularly dropping frames.

[Monitor]: (https://github.com/jwmerrill/elm-animation-frame)

In the existing implementation, it is possible to block rendering forever
using an external requestAnimationFrame loop by scheduling a new domUpdate for
the beginning of each animation frame. This cancels the previous draw callback
(using cancelAnimationFrame), and schedules a new draw callback that will not
run until the *next* frame. If a new domUpdate is requested in the next frame
also, the new draw callback will also be cancelled, and the process can repeat
arbitrarily long until new domUpdates stop being requested.

The new implementation does not use cancelAnimationFrame. Instead, it uses a
small state machine to ensure that only one drawCallback is ever scheduled per
frame.

The new implementation also drops the vendor prefixes on requestAnimationFrame
because [no recent browser uses them][1]. In case requestAnimationFrame is not
available, it uses `setTimeout(cb, 1000/60)`. The existing implementation used
an immediate call, but I think it's better to throttle draw calls to make the
behavior more similar to the case where requestAnimationFrame is available.

[1]: (https://developer.mozilla.org/en-US/docs/Web/API/window.requestAnimationFrame#Browser_compatibility)

A final small advantage of the new implementation is that, unlike the existing
implementation, it does not create a new function closure on every draw call.
@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Here's a picture of the draw scheduler state machine:

img_1392

Contributor

jwmerrill commented Dec 2, 2014

Here's a picture of the draw scheduler state machine:

img_1392

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

Why are we calling request animation frame when we know nothing needs to be drawn? I think the earlier version you shared avoided that, but is there some reason to churn at 60fps when we have a static webpage?

Member

evancz commented Dec 2, 2014

Why are we calling request animation frame when we know nothing needs to be drawn? I think the earlier version you shared avoided that, but is there some reason to churn at 60fps when we have a static webpage?

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

Hmm, I think I see. The idea is that if you are painting, the state should be DREW. If it is SCHEDULED, that implies we need to add a new frame. I'll read through again.

Member

evancz commented Dec 2, 2014

Hmm, I think I see. The idea is that if you are painting, the state should be DREW. If it is SCHEDULED, that implies we need to add a new frame. I'll read through again.

Show outdated Hide outdated src/Native/Runtime.js
return;
case DREW:
state = EMPTY;
return;

This comment has been minimized.

@evancz

evancz Dec 2, 2014

Member

It seems like this DREW case should never occur. How can we end up here and why is that a good thing?

@evancz

evancz Dec 2, 2014

Member

It seems like this DREW case should never occur. How can we end up here and why is that a good thing?

This comment has been minimized.

@jwmerrill

jwmerrill Dec 2, 2014

Contributor

This happens if you drew something on the last frame, and you have not asked to draw anything since. In this case, we stop the loop until a new draw call comes in.

@jwmerrill

jwmerrill Dec 2, 2014

Contributor

This happens if you drew something on the last frame, and you have not asked to draw anything since. In this case, we stop the loop until a new draw call comes in.

Show outdated Hide outdated src/Native/Runtime.js
"Please report this to <https://github.com/elm-lang/Elm/issues>."
);
case SCHEDULED:
_requestAnimationFrame(drawCallback);

This comment has been minimized.

@evancz

evancz Dec 2, 2014

Member

If there are no more animations around, why are we calling this now?

@evancz

evancz Dec 2, 2014

Member

If there are no more animations around, why are we calling this now?

This comment has been minimized.

@jwmerrill

jwmerrill Dec 2, 2014

Contributor

The reason for this is that when we draw, I want to schedule a check in the next frame that looks to see if there is anything new to draw (we have moved back to the SCHEDULED state) in which case we draw it, or if there is nothing new to draw (we are still in DREW), we move to the EMPTY state, which waits for another domUpdate request to schedule a new drawCallback.

@jwmerrill

jwmerrill Dec 2, 2014

Contributor

The reason for this is that when we draw, I want to schedule a check in the next frame that looks to see if there is anything new to draw (we have moved back to the SCHEDULED state) in which case we draw it, or if there is nothing new to draw (we are still in DREW), we move to the EMPTY state, which waits for another domUpdate request to schedule a new drawCallback.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

It seems like there are only two states, scheduled and waiting. It also seems like we have a queue of length 1, and we want to rAF whenever something is added to an empty queue.

Overall, something feels a bit off with the state machine here. Either it can be simplified, I am missing crucial information, or I just do not understand something.

Member

evancz commented Dec 2, 2014

It seems like there are only two states, scheduled and waiting. It also seems like we have a queue of length 1, and we want to rAF whenever something is added to an empty queue.

Overall, something feels a bit off with the state machine here. Either it can be simplified, I am missing crucial information, or I just do not understand something.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Here's my earlier implementation for comparison:

https://gist.github.com/jwmerrill/5a4c789d17380966a420

It looks simpler on the surface, but the reason I don't like it is line 37 in that gist:

https://gist.github.com/jwmerrill/5a4c789d17380966a420#file-elm-runtime-diff-L37

What that does is schedule one extra draw if we drew something new "just in case." Without that line, it's possible to end up only actually drawing on every other frame, and I understand the reason that that is true, but I find it hard to describe.

The new state machine avoids that fishy scene equality check.

Contributor

jwmerrill commented Dec 2, 2014

Here's my earlier implementation for comparison:

https://gist.github.com/jwmerrill/5a4c789d17380966a420

It looks simpler on the surface, but the reason I don't like it is line 37 in that gist:

https://gist.github.com/jwmerrill/5a4c789d17380966a420#file-elm-runtime-diff-L37

What that does is schedule one extra draw if we drew something new "just in case." Without that line, it's possible to end up only actually drawing on every other frame, and I understand the reason that that is true, but I find it hard to describe.

The new state machine avoids that fishy scene equality check.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

I agree that this looks a little more complicated than you might naively think it needs to be. This is the simplest and most rational form that I've found so far that covers all the edge cases.

The most important edge case is when you externally schedule a domUpdate in such a way that it is scheduled before drawCallback in any given frame during processing of the browser's requestAnimationFrame callback queue. Suppose I have a rule that says "if there's already a draw scheduled, don't schedule a new one". Then

  • In frame 1, I call domUpdate(scene1), but no draw occurs yet. Instead, a draw is scheduled for the next frame.
  • In frame 2, I call domUpdate(scene2), which does not schedule a new draw, because there is already one scheduled. Then drawCallback fires, and we actually draw scene2.
  • In frame 3, I call domUpdate(scene3), but there is no existing draw scheduled, and the new draw is scheduled for frame 4. We just dropped frame 3.
  • In frame 4, we're back to the position we were in in frame 2, and we actually draw scene4.

Things alternate like this on every other frame. This means that "don't schedule a new draw if a draw is already scheduled" is not a good strategy. This is also exactly the case that spoils the existing implementation--so "cancel existing draw callbacks whenever a new domUpdate is called" isn't a workable strategy either. Neither is "schedule a drawCallback on every domUpdate"--this is inefficient. And neither is "schedule a new drawCallback on every draw"--this makes you loop forever.

I'm not certain that 3 states are absolutely necessary--but do you have a different strategy in mind that handles the above edge case and is also simpler?

Contributor

jwmerrill commented Dec 2, 2014

I agree that this looks a little more complicated than you might naively think it needs to be. This is the simplest and most rational form that I've found so far that covers all the edge cases.

The most important edge case is when you externally schedule a domUpdate in such a way that it is scheduled before drawCallback in any given frame during processing of the browser's requestAnimationFrame callback queue. Suppose I have a rule that says "if there's already a draw scheduled, don't schedule a new one". Then

  • In frame 1, I call domUpdate(scene1), but no draw occurs yet. Instead, a draw is scheduled for the next frame.
  • In frame 2, I call domUpdate(scene2), which does not schedule a new draw, because there is already one scheduled. Then drawCallback fires, and we actually draw scene2.
  • In frame 3, I call domUpdate(scene3), but there is no existing draw scheduled, and the new draw is scheduled for frame 4. We just dropped frame 3.
  • In frame 4, we're back to the position we were in in frame 2, and we actually draw scene4.

Things alternate like this on every other frame. This means that "don't schedule a new draw if a draw is already scheduled" is not a good strategy. This is also exactly the case that spoils the existing implementation--so "cancel existing draw callbacks whenever a new domUpdate is called" isn't a workable strategy either. Neither is "schedule a drawCallback on every domUpdate"--this is inefficient. And neither is "schedule a new drawCallback on every draw"--this makes you loop forever.

I'm not certain that 3 states are absolutely necessary--but do you have a different strategy in mind that handles the above edge case and is also simpler?

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Ok, I think I see a way to simplify to 2 states now. Let me take a crack at that before you bother with additional commentary.

Contributor

jwmerrill commented Dec 2, 2014

Ok, I think I see a way to simplify to 2 states now. Let me take a crack at that before you bother with additional commentary.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

Okay, so my understanding is as follows:

  • Requesting a frame is likely to get pushed to two frames later for some internal reasons that are not clearly documented. Let's call this "the problem"
  • We can avoid this if we always schedule an "extra" rAF. Worst case, we call it an extra time but do very little work. Best case, we have a rAF ready to go so we can avoid "the problem".

If I understand correctly, maybe there are really three states.

Member

evancz commented Dec 2, 2014

Okay, so my understanding is as follows:

  • Requesting a frame is likely to get pushed to two frames later for some internal reasons that are not clearly documented. Let's call this "the problem"
  • We can avoid this if we always schedule an "extra" rAF. Worst case, we call it an extra time but do very little work. Best case, we have a rAF ready to go so we can avoid "the problem".

If I understand correctly, maybe there are really three states.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

I might name things NO_REQUEST, PENDING_REQUEST, PREEMPTIVE_REQUEST or something like that. Initially reading through, the name DREW did not indicate to me this frame skipping possibility.

It's crazy that we have to write an API for requesting animation frames around requestAnimationFrame...

Member

evancz commented Dec 2, 2014

I might name things NO_REQUEST, PENDING_REQUEST, PREEMPTIVE_REQUEST or something like that. Initially reading through, the name DREW did not indicate to me this frame skipping possibility.

It's crazy that we have to write an API for requesting animation frames around requestAnimationFrame...

jwmerrill added some commits Dec 2, 2014

Reduce draw scheduler state machine to 2 states.
This is transitional. A two state state-machine is just a boolean flag. But I
wanted to checkpoint here to make it clear how the implementations relate to
eachother.
Convert state machine to a single boolean.
drawScheduled: false corresponds to EMPTY
drawScheduled: true corresponses to SCHEDULED
@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Here's a picture of the new "state machine," that is in the end just implemented by a single boolean flag called drawScheduled:

img_1393

f4f277b is a lot closer to the gist version, but manages to get its work done without doing its own comparison of the scenes, which seemed fishy to me in this context.

Contributor

jwmerrill commented Dec 2, 2014

Here's a picture of the new "state machine," that is in the end just implemented by a single boolean flag called drawScheduled:

img_1393

f4f277b is a lot closer to the gist version, but manages to get its work done without doing its own comparison of the scenes, which seemed fishy to me in this context.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

So this one does not do the preemptive call to requestAnimationFrame. If you run things with this one, and the 3-state one, do you get the same amount of frame skips? If they are equivalent, then I think it makes sense to do it this way. But if I have understood things correctly, the preemptive rAF is actually giving us extra performance.

Member

evancz commented Dec 2, 2014

So this one does not do the preemptive call to requestAnimationFrame. If you run things with this one, and the 3-state one, do you get the same amount of frame skips? If they are equivalent, then I think it makes sense to do it this way. But if I have understood things correctly, the preemptive rAF is actually giving us extra performance.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Here's the relevant part of the spec, btw: http://www.w3.org/TR/animation-timing/#processingmodel

Clear as mud to me, but describing it in my own words and ignoring the multiple contexts part, it says something like:

  1. There is an animation frame request callback list. Whenever requestAnimationFrame is called, the callback is added to this list.
  2. Periodically, this list should be processed by moving its contents to a new list (the "working list" say), and then iterating through the new list, calling each callback unless it has been cancelled.

An important part of this is to realize that during processing of callbacks in the working list, new calls do not go to the end of the list you're iterating through (the working list), but rather on the animation frame request callback list.

Contributor

jwmerrill commented Dec 2, 2014

Here's the relevant part of the spec, btw: http://www.w3.org/TR/animation-timing/#processingmodel

Clear as mud to me, but describing it in my own words and ignoring the multiple contexts part, it says something like:

  1. There is an animation frame request callback list. Whenever requestAnimationFrame is called, the callback is added to this list.
  2. Periodically, this list should be processed by moving its contents to a new list (the "working list" say), and then iterating through the new list, calling each callback unless it has been cancelled.

An important part of this is to realize that during processing of callbacks in the working list, new calls do not go to the end of the list you're iterating through (the working list), but rather on the animation frame request callback list.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

I believe that the two state machines call requestAnimationFrame(drawCallback) and draw() at exactly the same times. The only difference is bookkeeping.

Contributor

jwmerrill commented Dec 2, 2014

I believe that the two state machines call requestAnimationFrame(drawCallback) and draw() at exactly the same times. The only difference is bookkeeping.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Alright, I have now convinced myself that the two state machines are not equivalent. The 1st one had the invariant that there would be at most one drawCallback scheduled at a time, and the new one does not maintain this invariant. I believe in the new version, you can have up to two drawCallback's scheduled if you move from SCHEDULED to EMPTY, and then from EMPTY back to SCHEDULED via a domUpdate that happens before the drawCallback is called.

I don't think that's a huge problem in practice, but it is an argument in favor of 645dc95 over f4f277b.

BTW, I'm not totally clear what you mean when you say "preemptive call to requestAnimationFrame."

Contributor

jwmerrill commented Dec 2, 2014

Alright, I have now convinced myself that the two state machines are not equivalent. The 1st one had the invariant that there would be at most one drawCallback scheduled at a time, and the new one does not maintain this invariant. I believe in the new version, you can have up to two drawCallback's scheduled if you move from SCHEDULED to EMPTY, and then from EMPTY back to SCHEDULED via a domUpdate that happens before the drawCallback is called.

I don't think that's a huge problem in practice, but it is an argument in favor of 645dc95 over f4f277b.

BTW, I'm not totally clear what you mean when you say "preemptive call to requestAnimationFrame."

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Sorry this has gotten so confusing. I spent some time stewing over this trying to get things clear and simple before I put up a PR, but apparently I wasn't finished figuring out how to understand it.

Contributor

jwmerrill commented Dec 2, 2014

Sorry this has gotten so confusing. I spent some time stewing over this trying to get things clear and simple before I put up a PR, but apparently I wasn't finished figuring out how to understand it.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

I think it has clarified things for me :) I now believe there are three states.

When I say "preemptive call to requestAnimationFrame" I mean that we are calling rAF even though we don't know we will need it. Maybe we will maybe we won't. That's the part that was objectionable to me at first, but I did not understand the frame skipping which you have explained quite well here.

So, I'm gonna say, let's go with the version in this PR and I'll try to do some names and comments such that I understand what everything is doing.

Awesome work figuring this out! :D

Member

evancz commented Dec 2, 2014

I think it has clarified things for me :) I now believe there are three states.

When I say "preemptive call to requestAnimationFrame" I mean that we are calling rAF even though we don't know we will need it. Maybe we will maybe we won't. That's the part that was objectionable to me at first, but I did not understand the frame skipping which you have explained quite well here.

So, I'm gonna say, let's go with the version in this PR and I'll try to do some names and comments such that I understand what everything is doing.

Awesome work figuring this out! :D

evancz pushed a commit that referenced this pull request Dec 2, 2014

Merge pull request #25 from jwmerrill/robust-draw-loop
Make draw scheduling more robust.

@evancz evancz merged commit e8a347d into elm:master Dec 2, 2014

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

Ahhh, oops, I did not realize the changes were applied in the PR already.

Member

evancz commented Dec 2, 2014

Ahhh, oops, I did not realize the changes were applied in the PR already.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

@evancz it looks like you merged all 3 commits from this PR, but what you said makes it seem to me like you intended to merge only the first (the 3 state version).

Looking more closely, I think the two state version is bad. I think it allows the number of pending callbacks to grow not just to 2, but by 1 on each frame in the worst case.

Contributor

jwmerrill commented Dec 2, 2014

@evancz it looks like you merged all 3 commits from this PR, but what you said makes it seem to me like you intended to merge only the first (the 3 state version).

Looking more closely, I think the two state version is bad. I think it allows the number of pending callbacks to grow not just to 2, but by 1 on each frame in the worst case.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

Yeah, I reverted back to the original PR. I think the last thing is about the fallback for rAF. You have a delay of 16ms, but I don't think that's so good. That means you definitely skip a frame every time, which seems weird to me. I get it in the case of animation, but the more common case is a mouse click or key press or something. I don't think it really makes sense to wait a frame there.

Member

evancz commented Dec 2, 2014

Yeah, I reverted back to the original PR. I think the last thing is about the fallback for rAF. You have a delay of 16ms, but I don't think that's so good. That means you definitely skip a frame every time, which seems weird to me. I get it in the case of animation, but the more common case is a mouse click or key press or something. I don't think it really makes sense to wait a frame there.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

Re: 16ms, you might be right. setTimeout(cb, 0) is another reasonable choice, which at least throttles many draw calls if they are called synchronously, but just calling the callback immediately is probably fine too.

Honestly, I think any of these choices are fine. Elm seems early enough along its adoption curve that most time should be focused on where browsers will be in the near future, rather than where the existing stragglers are.

Contributor

jwmerrill commented Dec 2, 2014

Re: 16ms, you might be right. setTimeout(cb, 0) is another reasonable choice, which at least throttles many draw calls if they are called synchronously, but just calling the callback immediately is probably fine too.

Honestly, I think any of these choices are fine. Elm seems early enough along its adoption curve that most time should be focused on where browsers will be in the near future, rather than where the existing stragglers are.

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Dec 2, 2014

Contributor

I'll mention one more thing. The indentation inside initGraphics was driving me nuts while I was editing it. It has lots of 4 space indents that are not aligned to 4 space boundaries (caused by nesting 2 space indents and 4 space indents). I would vote for choosing either 2 space indents or 4 space indents and using that everywhere. 2 space indents is my personal preference for JS, but I don't really mind what you use as long as it's consistent.

I'm happy to put up a separate PR to fix the whitespace in this file if you like, but I didn't want to mess with it in this PR.

Contributor

jwmerrill commented Dec 2, 2014

I'll mention one more thing. The indentation inside initGraphics was driving me nuts while I was editing it. It has lots of 4 space indents that are not aligned to 4 space boundaries (caused by nesting 2 space indents and 4 space indents). I would vote for choosing either 2 space indents or 4 space indents and using that everywhere. 2 space indents is my personal preference for JS, but I don't really mind what you use as long as it's consistent.

I'm happy to put up a separate PR to fix the whitespace in this file if you like, but I didn't want to mess with it in this PR.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Dec 2, 2014

Member

This file is sort of an amalgam of different files written at different times and by different people, so the style reflects that. I have been going for 4-space indent in new JS code I write, and I'd say everything is free game except the F2, F3, etc. functions. I'd like to use roughly Google's style for objects and ternary expressions:

var bar = {
    x: 3,
    y: 4
};

var foo = bool
    ? trueBranch
    : falseBranch
    ;
Member

evancz commented Dec 2, 2014

This file is sort of an amalgam of different files written at different times and by different people, so the style reflects that. I have been going for 4-space indent in new JS code I write, and I'd say everything is free game except the F2, F3, etc. functions. I'd like to use roughly Google's style for objects and ternary expressions:

var bar = {
    x: 3,
    y: 4
};

var foo = bool
    ? trueBranch
    : falseBranch
    ;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment