Skip to content
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

Wrap drawing in requestAnimationFrame. #1105

Merged
merged 7 commits into from
Jun 1, 2017
Merged

Wrap drawing in requestAnimationFrame. #1105

merged 7 commits into from
Jun 1, 2017

Conversation

agamemnus
Copy link
Contributor

@agamemnus agamemnus commented May 27, 2017

Wrap drawing in requestAnimationFrame per #1084.

Blocked by #1107.

@agamemnus
Copy link
Contributor Author

I guess this should be merged with the other one.

@katspaugh
Copy link
Owner

katspaugh commented May 29, 2017

Can you somehow add those requestAnimationFrames on top of the existing function calls?

I.e. instead of

drawSomething: function() {
  requestAnimationFrame(function () {
    // lots of indented code
  });
}

This:

_drawSomething: function() {
  // the original unindented code
  });
},

drawSomething: function() {
  var my = this;
  requestAnimationFrame(function () {
    my._drawSomething();
  });
}

@agamemnus
Copy link
Contributor Author

agamemnus commented May 29, 2017 via email

@katspaugh
Copy link
Owner

katspaugh commented May 29, 2017

Another approach would be to wrap the definitions of the methods:

utils.frame = function (callback) {
  return function () {
    var my = this;
    var args = arguments;

    requestAnimationFrame(function () {
      callback.apply(my, args);
    });
  };
}

drawSomething: utils.frame(function () {
  // the original unindented code
})

@agamemnus
Copy link
Contributor Author

agamemnus commented May 29, 2017 via email

@agamemnus
Copy link
Contributor Author

Idk what's up.

@katspaugh
Copy link
Owner

You spread your changes over several PRs and in this PR, there's no frame.

@agamemnus
Copy link
Contributor Author

Ah, okay. Idk how to combine them in the web gui.

@katspaugh
Copy link
Owner

Let's first merge the PR where you introduce these new utils.

@katspaugh
Copy link
Owner

Thank you!

@katspaugh katspaugh merged commit bc57ec8 into katspaugh:master Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants