Skip to content

Conversation

@rozele
Copy link
Contributor

@rozele rozele commented Dec 8, 2021

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Why

Currently, NativeAnimated compiles the animation graph into CompositionAnimations from UI.Composition. While this approach likely provides the ideal performance for native animations on Windows, it suffers a few insurmountable limitations:

  1. Native animation gets "stuck" when restarting animation before previous animation completes #8419: Creating a new animation on an animated value after stopping a previous animation on the same value does not retain the current value.
    a. Even with a fix like Fixes issue with restarting animations #9190, because UI.Composition values cannot be queried synchronously, starting a new animation on an animated value immediately after stopping a previous animation causes jitter in the animation, because it takes 1-2 frames for the completion callback to fire, signaling that the animated value has an up-to-date value.
  2. StringToFacadeType missing "progress" #3283: UI.Composition can only animate supported properties, like opacity and transforms. NativeAnimated provides a prop "hook" (the prop name is progress) to allow arbitrary views to subscribe to animation value changes synchronously. This is not possible with UI.Composition.
  3. Animated.Value cannot have a listener attatched to it #4311: Similar to the limitation for starting new animations synchronously after stopping a previous animation on the same animated value (Native animation gets "stuck" when restarting animation before previous animation completes #8419), animated value listeners will always be 1-2 frames out of sync while waiting for an up-to-date composition value. This feature is currently not implemented for the UI.Composition approach, but I suspect it would require a frame callback via CompositionTarget::Rendering, so not only would the values be out of sync, but the approach would have a similar performance profile to the CompositionTarget::Rendering driven animations.
  4. Unable to animate arbitrary style props with NativeAnimated #9255: Similar to StringToFacadeType missing "progress" #3283, it's actually possible to animate arbitrary props with Animated (e.g., borderRadius). It's unlikely that it would be possible to support such an animation with UI.Composition.

There are also a few bugs that are likely possible to workaround for UI.Composition, but would be fixed immediately by this CompositionTarget::Rendering approach:

  1. Native driver persists animated values after they're no longer applied #3460: Animated values persist even after the animation is unmounted with UI.Composition. The CompositionTarget::Rendering approach uses the view's shadow node as the source of truth for the prop value, and resets the prop to null when the animation is unmounted.
  2. NativeAnimated commandeers the animated value offset #9256: The UI.Composition implementation for NativeAnimated commandeers the Offset value for animated nodes, using it to drive progress on an animation. Animated value offsets are not intended for this purpose and can cause bugs as demonstrated in the linked issue.
  3. Calling setValue in NativeAnimated does not update the state correctly #9251: Calling setValue on an animated value should stop any active animations, but currently does not in the UI.Composition implementation.
  4. Animated.diffClamp does not account for diff in NativeAnimated #9252: Animated.diffClamp nodes are intended to clamp the difference between the last value and the current value, but the UI.Composition clamp expression only has the capability to clamp the current value.
  5. NativeAnimated getValue returns incorrect result #9250: Calling getValue on an animated value does not return the current value for the UI.Composition approach (because values are not available until the animation has been stopped and the completion callback fires).
  6. A more minor issue that I haven't filed an issue for, is that Animated.decay animations work slightly differently in NativeAnimated vs. JS-driven animations. The NativeAnimated approach is a bit more "accurate", in that it stops the animation when its value is within 0.1 of the final value if the decay ran infinitely. The JS-driven approach stops the animation more eagerly when the value differential (the difference between the current value and the previous value) is 0.1.

Resolves #3283
Resolves #3460
Resolves #4311
Resolves #5958
Resolves #8419
Resolves #9250
Resolves #9251
Resolves #9252
Resolves #9255
Resolves #9256

What

This change allows each animation node and driver to be used in either composition mode or frame callback / CompositionTarget::Rendering mode. The latter approach is largely derived from the Android implementation of NativeAnimated (and the C# implementation in react-native-windows v0.59 and earlier).

This change will leverage WIP changes to the Animated APIs in RN core that pass a property bag to each AnimationDriver and AnimatedNode signaling which mode to use. The API surface will look something like the following:

const value = Animated.value(0);

Animated.timing(value, {
  ...,
  useNativeDriver: true,
  platformConfig: {
    useComposition: false,
  });

We will also likely complement this API change with a way to set default platformConfig values for all Animated APIs using useNativeDriver: true:

Animated.unstable_setDefaultPlatformConfig({useComposition: false});

For now, in order to not regress performance on existing react-native-windows applications, using the CompositionTarget::Rendering approach will be strictly opt-in. As of this commit, these two approaches cannot be blended together. I.e., you cannot connect a node using UI.Composition to a node using CompositionTarget::Rendering, and it's unlikely these two approaches could be combined until the UI.Composition approach supports synchronous queries of values (at which point, quite a few of the justifications for the CompositionTarget::Rendering approach will be resolved).

Testing

There is no change in the animation behavior before and after this change, as this just sets up for the option to configure the animation type.

Before:

React.Native.Playground.Win32.2021-12-15.13-20-38.mp4

After:

React.Native.Playground.Win32.2021-12-15.13-16-50.mp4
Microsoft Reviewers: Open in CodeFlow

Here is a video of composition bugs that are mitigated with tick based animations:

React.Native.Playground.Win32.2022-08-02.16-56-37.mp4

Notes

Please note, this change leaves the NativeAnimated behavior effectively unchanged - i.e., native animations still default to UI.Composition, and there is no public API surface to allow users to force the CompositionTarget::Rendering driver until #9285 is merged.

@rozele rozele requested a review from a team as a code owner December 8, 2021 16:51
@rozele rozele marked this pull request as draft December 8, 2021 16:52
@ghost ghost added the Area: Animation label Dec 8, 2021
@rozele rozele force-pushed the nativeAnimatedRendering branch 6 times, most recently from 6b611cd to 1632903 Compare December 9, 2021 22:06
@rozele
Copy link
Contributor Author

rozele commented Dec 10, 2021

FYI - if we decide to move forward with this, I'm working on this PR to RN core, which has good support so far: facebook/react-native#32736, which would allow us to merge without forking so many files from Libraries/Animated in core RN. We'd only need to fork AnimatedPlatformConfig to fix the flow type (assuming that works) and to set the default value we wanted - though we could always set the default value natively.

@rozele rozele force-pushed the nativeAnimatedRendering branch 5 times, most recently from 7b133e7 to 46313a5 Compare December 14, 2021 19:49
@rozele rozele force-pushed the nativeAnimatedRendering branch 2 times, most recently from 6aa2fab to 0ce1881 Compare December 15, 2021 16:50
@rozele rozele changed the title DRAFT: Adds Rendering driver option to NativeAnimated Adds Rendering driver option to NativeAnimated Dec 15, 2021
@rozele rozele marked this pull request as ready for review December 15, 2021 16:51
@rozele rozele changed the title Adds Rendering driver option to NativeAnimated [NativeAnimated][2/3] Adds Rendering driver option to NativeAnimated Dec 15, 2021
@rozele rozele force-pushed the nativeAnimatedRendering branch 4 times, most recently from e363bed to e2933e4 Compare December 15, 2021 17:40
@rozele rozele force-pushed the nativeAnimatedRendering branch 3 times, most recently from 820a468 to 922c914 Compare December 15, 2021 17:53
@rozele rozele force-pushed the nativeAnimatedRendering branch from 237719b to e0d3726 Compare December 16, 2021 20:36
@rozele rozele force-pushed the nativeAnimatedRendering branch 2 times, most recently from 1ee5280 to 4d8d0a4 Compare January 11, 2022 21:36
@rozele rozele force-pushed the nativeAnimatedRendering branch 5 times, most recently from e163f44 to dc18593 Compare January 24, 2022 22:02
@rozele rozele force-pushed the nativeAnimatedRendering branch 3 times, most recently from db4616e to 14e123f Compare June 17, 2022 19:29
@rozele rozele changed the title [NativeAnimated][1/3] Adds Rendering driver option to NativeAnimated Adds Rendering driver option to NativeAnimated Jun 17, 2022
@rozele rozele force-pushed the nativeAnimatedRendering branch 2 times, most recently from b4151b3 to 1c09d6c Compare August 3, 2022 16:39
rozele added a commit to rozele/react-native-windows that referenced this pull request Oct 23, 2022
Currently, NativeAnimated compiles the animation graph into
CompositionAnimations from UI.Composition. While this approach likely
provides the ideal performance for native animations on Windows, it
suffers a few insurmountable limitations:

1. microsoft#8419: Creating a new animation on an animated value after stopping a
previous animation on the same value does not retain the current value.
  a. Even with a fix like microsoft#9190, because UI.Composition values cannot be
  queried synchronously, starting a new animation on an animated value
  immediately after stopping a previous animation causes jitter in the
  animation, because it takes 1-2 frames for the completion callback to
  fire, signaling that the animated value has an up-to-date value.
2. microsoft#3283: UI.Composition can only animate supported properties, like
opacity and transforms. NativeAnimated provides a prop "hook" (the prop
name is `progress`) to allow arbitrary views to subscribe to animation
value changes synchronously. This is not possible with UI.Composition.
3. microsoft#4311: Similar to the limitation for starting new animations
synchronously after stopping a previous animation on the same animated
value (microsoft#8419), animated value listeners will always be 1-2 frames out of sync
while waiting for an up-to-date composition value. This feature is
currently not implemented for the UI.Composition approach, but I suspect
it would require a frame callback via CompositionTarget::Rendering, so
not only would the values be out of sync, but the approach would have a
similar performance profile to the CompositionTarget::Rendering driven
animations.
4. microsoft#9255: Similar to microsoft#3283, it's actually possible to animate arbitrary
props with Animated (e.g., `borderRadius`). It's unlikely that it would
be possible to support such an animation with UI.Composition.

There are also a few bugs that are likely possible to workaround for
UI.Composition, but would be fixed immediately by this
CompositionTarget::Rendering approach:
1. microsoft#3460: Animated values persist even after the animation is unmounted
with UI.Composition. The CompositionTarget::Rendering approach uses the
view's shadow node as the source of truth for the prop value, and resets
the prop to null when the animation is unmounted.
2. microsoft#9256: The UI.Composition implementation for NativeAnimated
commandeers the `Offset` value for animated nodes, using it to drive
progress on an animation. Animated value offsets are not intended for this
purpose and can cause bugs as demonstrated in the linked issue.
3. microsoft#9251: Calling `setValue` on an animated value should stop any active
animations and update the animation graph to reflect the value that was
set, but in the UI.Composition implementation, the animation is only
stopped in place.
4. microsoft#9252: Animated.diffClamp nodes are intended to clamp the difference
between the last value and the current value, but the UI.Composition
`clamp` expression only has the capability to clamp the current value.
5. microsoft#9250: Calling `getValue` on an animated value does not return the
current value for the UI.Composition approach (because values are not
available until the animation has been stopped and the completion
callback fires).
6. A more minor issue that I haven't filed an issue for, is that
Animated.decay animations work slightly differently in NativeAnimated
vs. JS-driven animations. The NativeAnimated approach is a bit more
"accurate", in that it stops the animation when its value is within 0.1
of the final value if the decay ran infinitely. The JS-driven approach
stops the animation more eagerly when the value differential (the
difference between the current value and the previous value) is 0.1.

This change allows each animation node and driver to be used in either
composition mode or frame callback / CompositionTarget::Rendering mode.
The latter approach is largely derived from the Android implementation
of NativeAnimated (and the C# implementation in react-native-windows
v0.59 and earlier).

This change will leverage WIP changes to the Animated APIs in RN core
that pass a property bag to each AnimationDriver and AnimatedNode
signaling which mode to use. The API surface will look something like
the following:

```js
const value = Animated.value(0);

Animated.timing(value, {
  ...,
  useNativeDriver: true,
  platformConfig: {
    useComposition: false,
  });
```

We will also likely complement this API change with a way to set default
`platformConfig` values for all Animated APIs using `useNativeDriver:
true`:
```js
Animated.setDefaultPlatformConfig({useComposition: false});
```

For now, in order to not regress performance on existing
react-native-windows applications, using the
CompositionTarget::Rendering approach will be strictly opt-in. As of
this commit, these two approaches cannot be blended together. I.e., you
cannot connect a node using UI.Composition to a node using
CompositionTarget::Rendering, and it's unlikely these two approaches
could be combined until the UI.Composition approach supports synchronous
queries of values (at which point, quite a few of the justifications for the
CompositionTarget::Rendering approach will be resolved).

Fixes microsoft#3283
Fixes microsoft#3460
Fixes microsoft#8419
Fixes microsoft#9250
Fixes microsoft#9251
Fixes microsoft#9252
Fixes microsoft#9255
Fixes microsoft#9256
@agniuks
Copy link
Contributor

agniuks commented Nov 3, 2022

FYI - if we decide to move forward with this, I'm working on this PR to RN core, which has good support so far: facebook/react-native#32736, which would allow us to merge without forking so many files from Libraries/Animated in core RN. We'd only need to fork AnimatedPlatformConfig to fix the flow type (assuming that works) and to set the default value we wanted - though we could always set the default value natively.

We'll be reviewing this over the coming week -- curious now that the PR in core RN has been merged, does this need any changes still to account for that? Also out of the merge conflicts, anything non-trivial there? @rozele

@rozele rozele force-pushed the nativeAnimatedRendering branch from 1c09d6c to 348ec47 Compare November 7, 2022 15:03
Currently, NativeAnimated compiles the animation graph into
CompositionAnimations from UI.Composition. While this approach likely
provides the ideal performance for native animations on Windows, it
suffers a few insurmountable limitations:

1. microsoft#8419: Creating a new animation on an animated value after stopping a
previous animation on the same value does not retain the current value.
  a. Even with a fix like microsoft#9190, because UI.Composition values cannot be
  queried synchronously, starting a new animation on an animated value
  immediately after stopping a previous animation causes jitter in the
  animation, because it takes 1-2 frames for the completion callback to
  fire, signaling that the animated value has an up-to-date value.
2. microsoft#3283: UI.Composition can only animate supported properties, like
opacity and transforms. NativeAnimated provides a prop "hook" (the prop
name is `progress`) to allow arbitrary views to subscribe to animation
value changes synchronously. This is not possible with UI.Composition.
3. microsoft#4311: Similar to the limitation for starting new animations
synchronously after stopping a previous animation on the same animated
value (microsoft#8419), animated value listeners will always be 1-2 frames out of sync
while waiting for an up-to-date composition value. This feature is
currently not implemented for the UI.Composition approach, but I suspect
it would require a frame callback via CompositionTarget::Rendering, so
not only would the values be out of sync, but the approach would have a
similar performance profile to the CompositionTarget::Rendering driven
animations.
4. microsoft#9255: Similar to microsoft#3283, it's actually possible to animate arbitrary
props with Animated (e.g., `borderRadius`). It's unlikely that it would
be possible to support such an animation with UI.Composition.

There are also a few bugs that are likely possible to workaround for
UI.Composition, but would be fixed immediately by this
CompositionTarget::Rendering approach:
1. microsoft#3460: Animated values persist even after the animation is unmounted
with UI.Composition. The CompositionTarget::Rendering approach uses the
view's shadow node as the source of truth for the prop value, and resets
the prop to null when the animation is unmounted.
2. microsoft#9256: The UI.Composition implementation for NativeAnimated
commandeers the `Offset` value for animated nodes, using it to drive
progress on an animation. Animated value offsets are not intended for this
purpose and can cause bugs as demonstrated in the linked issue.
3. microsoft#9251: Calling `setValue` on an animated value should stop any active
animations and update the animation graph to reflect the value that was
set, but in the UI.Composition implementation, the animation is only
stopped in place.
4. microsoft#9252: Animated.diffClamp nodes are intended to clamp the difference
between the last value and the current value, but the UI.Composition
`clamp` expression only has the capability to clamp the current value.
5. microsoft#9250: Calling `getValue` on an animated value does not return the
current value for the UI.Composition approach (because values are not
available until the animation has been stopped and the completion
callback fires).
6. A more minor issue that I haven't filed an issue for, is that
Animated.decay animations work slightly differently in NativeAnimated
vs. JS-driven animations. The NativeAnimated approach is a bit more
"accurate", in that it stops the animation when its value is within 0.1
of the final value if the decay ran infinitely. The JS-driven approach
stops the animation more eagerly when the value differential (the
difference between the current value and the previous value) is 0.1.

This change allows each animation node and driver to be used in either
composition mode or frame callback / CompositionTarget::Rendering mode.
The latter approach is largely derived from the Android implementation
of NativeAnimated (and the C# implementation in react-native-windows
v0.59 and earlier).

This change will leverage WIP changes to the Animated APIs in RN core
that pass a property bag to each AnimationDriver and AnimatedNode
signaling which mode to use. The API surface will look something like
the following:

```js
const value = Animated.value(0);

Animated.timing(value, {
  ...,
  useNativeDriver: true,
  platformConfig: {
    useComposition: false,
  });
```

We will also likely complement this API change with a way to set default
`platformConfig` values for all Animated APIs using `useNativeDriver:
true`:
```js
Animated.setDefaultPlatformConfig({useComposition: false});
```

For now, in order to not regress performance on existing
react-native-windows applications, using the
CompositionTarget::Rendering approach will be strictly opt-in. As of
this commit, these two approaches cannot be blended together. I.e., you
cannot connect a node using UI.Composition to a node using
CompositionTarget::Rendering, and it's unlikely these two approaches
could be combined until the UI.Composition approach supports synchronous
queries of values (at which point, quite a few of the justifications for the
CompositionTarget::Rendering approach will be resolved).

Fixes microsoft#3283
Fixes microsoft#3460
Fixes microsoft#8419
Fixes microsoft#9250
Fixes microsoft#9251
Fixes microsoft#9252
Fixes microsoft#9255
Fixes microsoft#9256
@rozele rozele force-pushed the nativeAnimatedRendering branch from 348ec47 to 5e55a55 Compare November 7, 2022 17:39
@rozele
Copy link
Contributor Author

rozele commented Nov 7, 2022

@AgneLukoseviciute - I just rebased this on top of @acoates-ms recent migration of NativeAnimated to a TurboModule, so it should be good to go on that front. There's nothing to account for from the PR in RN core. I have already removed the JS changes to Animated that this needed.

Copy link
Contributor

@chiaramooney chiaramooney left a comment

Choose a reason for hiding this comment

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

👍

@chiaramooney chiaramooney added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Nov 10, 2022
@ghost
Copy link

ghost commented Nov 10, 2022

Hello @chiaramooney!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 80be786 into microsoft:main Nov 10, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment