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

Add support for spring duration #4179

Merged
merged 27 commits into from
May 24, 2023

Conversation

Latropos
Copy link
Contributor

@Latropos Latropos commented Mar 7, 2023

Summary

We have decided to introduce two new config props - damping ratio and duration to make usage of withSpring easier.

  graph LR;
      A[Use<br>withSpring<br>animation] --> B;
      B{Did user provide<br>config with duration or<br>dampingRatio?} --YES-->C[Calculate spring movement using default or provided<br>mass and damping. Defaults:<br>damping 10, mass 1];
      B--NO-->D[Calculate spring movement using default or provided<br>duration and damping ratio. Defaults:<br>duration: 2, dampingRatio: 0.5];

Implementation

When starting an animation with predefined duration we have to find a mass that would give us the expected duration. Thus we have to solve a simple numeric equation. We do it with bisection, since Newton method appears to be much slower in this case (it happens, when finding roots of functions that grow really fast 📈)

Testing that Newton's method is much slower
Screen.Recording.2023-04-20.at.11.01.07.mov

Recordings

In all the examples below duration is set to 5000ms

dampingRatio=1dampingRatio=0.5
Screen.Recording.2023-04-20.at.12.11.53.mov

Screen.Recording.2023-04-20.at.12.13.11.mov
dampingRatio=1000dampingRatio=0.1
Screen.Recording.2023-04-20.at.12.14.11.mov
Screen.Recording.2023-04-20.at.12.15.20.mov

Test plan

Run demo

@Latropos Latropos marked this pull request as draft March 7, 2023 11:06
react-native-reanimated.d.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
@piaskowyk piaskowyk self-requested a review March 7, 2023 17:30
@Latropos Latropos force-pushed the @acynk/spring-duration branch 4 times, most recently from 24b600b to b56f8b5 Compare March 8, 2023 11:30
@piaskowyk piaskowyk marked this pull request as ready for review March 9, 2023 10:05
Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

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

For me is great! But let's wait with merge since kmagiera opinion.

Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

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

Oh, we forgot about typescript and about adding proper types to d.ts

@Latropos Latropos requested a review from piaskowyk March 13, 2023 09:29
@kmagiera
Copy link
Member

Discussed that with @piaskowyk today but wanted to post the summary anyways;

  1. We should allow for a few different configurations to work: a) duraiton + damping b) duration + stiffness c) just duration with some sensible default (preferably we should treat if someone has provided the default damping). The difference is that now we always override damping, so if user passes a damping parameter it'd get overridden and result in unexpected behavior. Note that damping is a parameter that's intuitive to use (e.g. critical damping results in no overshooting, underdamping results in the oscillator making swings and there are more swings the lower the parameter value).
  2. We should always make sure that if duration is provided the animation always finishes at that specified time. Currently we use position and velocity thresholds for finishing the animation but that may result in the time being a bit off dompared to the provided duration. For the case when duration is provided we should use a different condition and only look at elapsed time (similar to what we do in timing animations)

Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

Is it ok that we don't take velocity into account when calculating damping/mass?

src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

I wonder why we limit duration-based animation to underdamped case only (over-damped seem to have an easier mechanics). On top of that it looks like we are not handling the case of overriding an ongoing spring animation with duration. I'd suggest that you try and replace this line https://github.com/software-mansion/react-native-reanimated/blob/main/Example/src/AnimatedStyleUpdateExample.tsx#L20 with spring animation with duration – then when you tap the button while the animation is still running, in case of timing it will only change the destination but it will finish at the designated time. With spring animation it seem like the new animation will start from new point and on top of that velocity will be reset, so there will be no smooth transition between animation from one target to the other?

src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
@Latropos
Copy link
Contributor Author

@kmagiera

velocity will be reset, so there will be no smooth transition

Yes, but I don't think that average user will be able to see it. I've decided to set it to zero, since if we modify mass to change the duration we get this formula LINK. So unless $v_0$ is zero we have to use Lambert W function 😢 I can try to improve it for cases in which user did not provide default damping, since if we update damping instead of mass it gets easier [LINK]

I wonder why we limit duration-based animation to underdamped case only

Type of the motion depends on $\zeta$, and
$$\zeta = \frac{c}{2 \sqrt{km}}$$
It means we know $\zeta$ after we have calculated mass and damping, so

  1. If toValue is not predefined different animations with the same config could have different types. In case of real world spring type of motion never change, no matter how you change the amplitude
  2. We would have the following cycle: Damping depends on duration -> motion type depends on damping -> duration depends on motion type. So it would be much more difficult. Even if we can solve it, as you've noticed "over-damped seem to have an easier mechanics" so user can obtain such a result with some easing as well.

Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

This is getting very close, just a couple more comments I added inline

useAnimatedStyle,
Easing,
withSpring,
Copy link
Member

Choose a reason for hiding this comment

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

Can we please revert all the changes to this file. It was meant to be the most basic example of using animations inside uAS. We link to it in bunch of places. If we want, we can swap out timing with spring but don't make it any more sophisticated than that (i.e. don't add more components etc)

src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/springUtils.ts Outdated Show resolved Hide resolved
animation.current = newPosition;
animation.velocity = newVelocity;

if (isOvershooting || (isVelocity && isDisplacement)) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd been discussed somewhere before but perhaps got lost: we want for animations with duration set to have additional stop condition such that it ends at the exact moment in time. This is needed because duration could be used to synchronize things happening on screen and we need to make sure the animation finishes exactly when requested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been thinking you mean cases when the obtained animation is too long and we want to terminate it earlier, but it looks like you want to "animate a static string" as well. IMHO both situations are very different so I've added four options to specify exactDuration parameter, instead of a single boolean flag. 🤔

@Latropos Latropos requested a review from kmagiera May 16, 2023 09:28
@Latropos Latropos force-pushed the @acynk/spring-duration branch 2 times, most recently from d27bf6e to 7af0989 Compare May 17, 2023 13:08
src/reanimated2/animation/spring.ts Show resolved Hide resolved
t,
});

const { isOvershooting, isVelocity, isDisplacement } =
Copy link
Member

Choose a reason for hiding this comment

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

There seem to be no point in calling this method when duration is set. We should collocate the final condition for the case when duration is set and not set to make this cleaner

dampingRatio: 0.5,
} as const;

const config = { ...defaultConfig, ...userConfig };
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const config = { ...defaultConfig, ...userConfig };
const config = { ...defaultConfig, ...userConfig, useDuration: userConfig.duration || userConfig.dampingRatio };

src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/springUtils.ts Outdated Show resolved Hide resolved
src/reanimated2/animation/spring.ts Show resolved Hide resolved
@Latropos Latropos added this pull request to the merge queue May 24, 2023
Merged via the queue into software-mansion:main with commit b76fa64 May 24, 2023
6 checks passed
@Latropos Latropos deleted the @acynk/spring-duration branch May 24, 2023 10:54
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

We have decided to introduce two new config props - damping ratio and
duration to make usage of `withSpring` easier.

```mermaid
  graph LR;
      A[Use<br>withSpring<br>animation] --> B;
      B{Did user provide<br>config with duration or<br>dampingRatio?} --YES-->C[Calculate spring movement using default or provided<br>mass and damping. Defaults:<br>damping 10, mass 1];
      B--NO-->D[Calculate spring movement using default or provided<br>duration and damping ratio. Defaults:<br>duration: 2, dampingRatio: 0.5];
```

### Implementation
When starting an animation with predefined duration we have to find a
mass that would give us the expected duration. Thus we have to solve a
simple numeric equation. We do it with bisection, since Newton method
appears to be much slower in this case (it happens, when finding roots
of functions that grow really fast 📈)

<details><summary>Testing that Newton's method is much slower</summary>


https://user-images.githubusercontent.com/56199675/233330812-5eb31550-f77d-4b68-8765-5131ba18c2d1.mov
</details> 

## Recordings
In all the examples below duration is set to 5000ms

<table>
<tr><td>dampingRatio=1</td><td>dampingRatio=0.5</td></tr>
<tr><td>


https://user-images.githubusercontent.com/56199675/233336875-b2666629-f3e9-403e-b629-fd44efba0dea.mov</td><td>


https://user-images.githubusercontent.com/56199675/233337127-433077d3-2017-4fde-8c1e-647c0ab52600.mov

</td></tr>
<tr><td>dampingRatio=1000</td><td>dampingRatio=0.1</td></tr>
<tr><td>


https://user-images.githubusercontent.com/56199675/233337155-a8f07c34-d65e-4161-ba47-b8fc9992482a.mov

</td><td>


https://user-images.githubusercontent.com/56199675/233337200-67b1719e-5963-4e65-b187-551cf80966cf.mov

</td></tr>
</table>

## Test plan


Run demo

---------

Co-authored-by: Aleksandra Cynk <aleksandracynk@Aleksandras-MacBook-Pro.local>
Co-authored-by: Aleksandra Cynk <aleksandracynk@swmansion.com>
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

3 participants