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

Use Animated.loop(), when possible, to limit JS thread utilization #204

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jamesreggio
Copy link

This PR will use Animated.loop(), when possible, to run looped animations. When combined with useNativeDriver, this prevents the JS thread from having to kick off each successive loop of an animation, making this library much more suitable for loading spinners and other animations that loop infinitely while the device is under heavy load.

If it means anything, I've been using this code in our production app for the past six months with no issue.

@jamesreggio
Copy link
Author

Friendly ping.

animation = Animated.loop(animation, loopConfig);
}

animation.start(endState => {
currentIteration += 1;
Copy link
Owner

Choose a reason for hiding this comment

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

I'd prefer to rewrite this completely to only use Animated.loop to make it less complicated. For infinite loops you can pass -1 as iterations.

Copy link
Author

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but I don't think you can support iterationDelay using Animated.loop.

I'd be happy to adjust the code so that single iterations also utilize Animated.loop, and that infinite loops use -1, but I don't think we can entirely eliminate this conditional.

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, wonder if we can solve this with Animated.delay() instead, but probably not.

Copy link
Author

Choose a reason for hiding this comment

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

I took a more careful look and it appears that simply applying iterationDelay within the Animated.timing works fine in the context of an Animated.loop.

I've updated the PR and I think you'll find it much more succinct (and thus to your liking).

@DomiR
Copy link

DomiR commented Sep 13, 2018

👍

@jamesreggio
Copy link
Author

@oblador — just in case you didn't see it, I heeded your feedback and simplified the implementation.

@jamesreggio
Copy link
Author

Ping, @oblador.

@hirbod
Copy link

hirbod commented Dec 1, 2019

Will this ever land?

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.

None yet

4 participants