Updated Interpolation example and fix rare crash #411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed mounting example, not very interesting - Added interpolation between new paths to better support use cases - Fixed bug with runTiming not releasing the animation value after it is done - which causes crashes in the native code after a while.
wcandillon
reviewed
Apr 20, 2022
wcandillon
reviewed
Apr 20, 2022
| @@ -27,7 +27,11 @@ export const runTiming = ( | |||
| callback?: AnimationCallback | |||
| ): SkiaAnimation => { | |||
| const resolvedParameters = getResolvedParams(toOrParams, config); | |||
| const animation = createTiming(resolvedParameters, value, callback); | |||
| const resolvedCallback = (current: number) => { | |||
There was a problem hiding this comment.
I appear to be able to reproduce the crash on iOS even on this branch, could that be? 🤔
- Removed mounting example, not very interesting - Added interpolation between new paths to better support use cases - Fixed bug with runTiming not releasing the animation value after it is done - which causes crashes in the native code after a while.
chrfalch
force-pushed
the
feature/updated-path-interpolation
branch
from
dfc52f7
to
f3ff1e6
Compare
…om/Shopify/react-native-skia into feature/updated-path-interpolation
…om/Shopify/react-native-skia into feature/updated-path-interpolation
… method on the JS thread. Used the trick where we use shared_from_this() and pass it to the lambda to make sure we keep alive until the dispatch method is finished. Also added automatic interpolation in demo to be able to reproduce error. Also made destructors virtual since the class has virtual members.
|
very nice |
wcandillon
reviewed
Apr 20, 2022
| // We need to stop/unsubscribe | ||
| stopClock(); | ||
| } | ||
| virtual ~RNSkAnimation() {} |
…e alive as long as any lambdas are running.
…om/Shopify/react-native-skia into feature/updated-path-interpolation
wcandillon
approved these changes
Apr 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To better support path interpolation use cases I updated the example to interpolate between paths when paths are changed - meaning that whenever we set a new path the component will animate between the previous and the new path. The example also supports interruptible animation.
Also fixed a bug in the runTiming function where the value.animation property was not reset when the animation finished.
Changes in this PR:
Fixes #414