Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34927
The changes made in D36902220 (https://github.com/facebook/react-native/commit/a04195167bbd8f27c6141c0239a61a345cac5a88) and D36958882 (https://github.com/facebook/react-native/commit/d8c25ca1b62df2b93f70bbb1f7b379643ab9ccd4) attempted to reduce flickering and consistency issues when using Animated.
In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?).
Changelog:
[General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer
Fixes#34665
Reviewed By: rshest
Differential Revision: D40194072
fbshipit-source-id: 1b3fb1d1f4a39036a501a8a21e57002035dd5659
# Conflicts:
# Libraries/Animated/createAnimatedComponent.js
Summary:
Adds support for handling animations in response to events on the platform side, without needing a JS round trip. With this TM, NativeViewEvents will no longer affected by JS thread - hover events will not be delayed when JS thread is busy. This makes a significant difference for VR panel apps - see test plan for an example for the before and after in Store.
Changelog:
[Internal] - Make NativeAnimatedNodesManager public
Reviewed By: JoshuaGross
Differential Revision: D37082069
fbshipit-source-id: 330acd78c547587de5545b61895e0d821fb99552
Summary:
This op code was incorrectly configured to take two args, while it only takes one.
Changelog: [Internal]
Differential Revision: D36664590
fbshipit-source-id: 6e1fdb9f64bbd32fbe05bbd174f94ae57292bcf9
Summary:
We were calling the wrong operation from the MegaOp which caused indices to become offset. Trivial fix.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D36482822
fbshipit-source-id: 83ebc37a8773f0277db4caff9d3e7c9c91931ddb
Summary:
Which idiot wrote this? Oh yeah, I did.
This was introduced in D36109810 (https://github.com/facebook/react-native/commit/bf405d70837e1319cfa83e4c5cbb7c9a69abd820) so hasn't been around too long luckily.
Changelog: [Internal]
Reviewed By: cortinico, GijsWeterings
Differential Revision: D36480272
fbshipit-source-id: b0fbb6b820185185359c6c75f77f2921dc892b02
Summary:
We introduce a few optimizations:
(1) Previous diff: We defer calling any NativeAnimatedModule methods by waiting 1ms before flushing the queue, and debouncing until no flush is requested. Practically, this just means that we'll call NativeAnimatedModule methods N times at once, at the end of a render loop, instead of N times smeared throughout the render loop.
(2) Additionally, instead of calling N methods, we create multi-operation argument buffer and call a single NativeAnimatedModule API, which should essentially throttle NativeAnimatedModule API calls to once-ish per frame. On the native side, this also reduces a lot of overhead associated with scheduling work on the UI thread (we schedule 1 function to run on the UI thread and perform N operations, as opposed to scheduling N functions to run on the UI thread).
TODO:
- implement stubs for iOS
- write gating code so this can be properly tested in VR and in fb4a
Changelog: [Internal]
Reviewed By: genkikondo
Differential Revision: D36338606
fbshipit-source-id: 29ac949b53b874683128a76525586c22def3143b
Summary:
Noticed that (LockFree)EventDispatcherImpl had NativeAnimatedNodesManager as a listener 100+ times, as listener registration can happen multiple times from Animated node creation. Since listener management on event dispatcher is thread-safe, we can avoid the thread hop for this.
Changelog: [Internal]
Reviewed By: genkikondo
Differential Revision: D36316102
fbshipit-source-id: f2f417b69885def87f88460d8b1e0b35b66726cb
Summary:
Noticed in D36098912 (https://github.com/facebook/react-native/commit/5de0f145eca36d3263cb8a9928add278cdfffa2b) that the definition of NativeAnimatedModule's OperationQueue.isEmpty was incorrect as it didn't account for `mPeekedOperation`, while `drainQueueIntoList` did.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D36109810
fbshipit-source-id: afd7059975ce6e5c65d1975d168a03f0881bd7c1
Summary:
Blocks on queue write/drain for Animated module under a feature flag to test whether it resolves race conditions.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D34752947
fbshipit-source-id: a1b1a286772d29a7a27b5e9c3f743cac84cc2bab
Summary:
Changes `peek`->`poll` on Animated node queues to a single `poll`, accounting for race conditions. In case the queue batch is split at some point, previously polled operation is kept as a "carry-over" inside the queue abstraction.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D34377737
fbshipit-source-id: 78e5ca60c4777576fbf009c3d3ffa53629221910
Summary:
The concurrent queue seems to leak values on some Android 12 devices when interleaving instruction polling and execution. To help with that, we can drain the queue before executing operations, reducing the window when `add` and `poll` happen concurrently.
Changelog: [Internal] - Drain the Animated queue before execution
Reviewed By: JoshuaGross
Differential Revision: D34370056
fbshipit-source-id: 781631dbf1a10e3f1ba9564db985403bd77a2c06
Summary:
In order to support AnimatedColor.setValue for platform colors, we need to pass the platform color object to the native animated node which will then resolve and apply the color.
Thus, the approach is:
- Add a new API updateAnimatedNodeConfig to NativeAnimatedModule
- [JS] On AnimatedColor.setValue, if the value is a platform color, then we call updateAnimatedNodeConfig
- [Android] We introduce AnimatedNodeWithUpdateableConfig interface with a method updateConfig. On ColorAnimatedNode.java, we use updateConfig to resolve and apply the color
Changelog:
[Internal][Fixed] - Use context from view when resolving platform color
Reviewed By: javache, mdvacca
Differential Revision: D34025193
fbshipit-source-id: 8b368f6b7cb2cf7cebe8b66461cd4185cbadd44c
Summary:
ReactSoftException makes it seem like the class is an Exception, when it's actually a logger. This rename I think is more appropriate.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D30251735
fbshipit-source-id: 550bd543388e698774ec31753200bd3f722d8c17
Summary:
This change broke some animations on non-Fabric surfaces due to inconsistent batching of animation operations.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D28013968
fbshipit-source-id: 2f65c799dbe00168f1e756ef0af60206df5a8fcc
Summary:
This change caused crashes in animations on some surfaces.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D28013969
fbshipit-source-id: 95845c69d6e67d59582ea14ad08cbf42fd3e2f8f
Summary:
D27682424 (https://github.com/facebook/react-native/commit/ea1ff374de2ece7d1698b14d4e1aa8075df22cdd) updated how animated node batches are executed in Fabric. On Paper, these batches were controlled by native module in some places (batch was executed ~every 2 frames), but some animations were switching animation batching control to JS globally there as well.
This change updates two things:
- If batching is controlled by native, it makes sure batches are calculated correctly.
- At the same time, this change switches control for animation node batching to JS, aligning it with Fabric.
Changelog: [Internal]
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D27939659
fbshipit-source-id: d6251bce2a303a4a007dc10297edc0175cc4b348
Summary:
Changelog: [Internal]
`NativeAnimatedModule` on Android currently enforces all animation operations to be processed in batches to ensure that all associated operations are processed at the same time.
Some operations, however, can be triggered outside of the batching calls (e.g. when using `Animated` for tracking touches `PanResponder`), and they are not processed until the next batch.
This change tracks if we are currently processing a batch and doesn't assign a batch number if an operation was triggered outside of `startOperationBatch`/`finishOperationBatch` pair.
Reviewed By: mdvacca
Differential Revision: D27682424
fbshipit-source-id: 2ea8737c353c81557fa586b15aa5760db3e8813f
Summary:
This diff migrates all NativeModules away from onCatalystInstanceDestroy() to the invalidate() method.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D26871595
fbshipit-source-id: 132f6b75e485361835769a2b53bc742eefa47b59
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859158
fbshipit-source-id: 4966d3c49d194c4cb4063edf3a035f6077b76cd9
Summary:
When switching between non-Fabric and Fabric screens, I believe that `initializeEventListenerForUIManagerType` is not always being called on the NativeAnimatedNodesManager if
`NativeAnimatedModule.initializeLifecycleEventListenersForViewTag` is being called before the NativeAnimatedNodesManager ivar has been set. This should occur very infrequently, but logs
indicate that it /does/ happen in some marginal cases. Protecting against these cases should be trivial, by using the `getNodesManager` method which is responsible for returning a nodes manager or creating a new one.
The existing uses of the `NativeAnimatedNodesManager` ivar also occur on different threads and we were not protecting against this, so I'm changing it to an atomic. It's very likely that
the inconsistency issues in the past were caused not by ordering errors, but thread races.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24267118
fbshipit-source-id: 68abbff7ef3d0b2ecc9aa9977165663ad9447ab8
Summary:
In the past I tried a few heuristics to guess when a batch of Animated Operations were ready, and none of these were super reliable. But it turns out we can safely allow JS to manage that explicitly.
Non-Fabric still uses the old behavior which seems fine.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23010844
fbshipit-source-id: 4c688d3a61460118557a4971e549ec7457f3eb8f
Summary:
After D22801173 (https://github.com/facebook/react-native/commit/9e6ba9ddb8608d4e3a4a80d0138600130b766d4c) has landed, the native mechanisms in NativeAnimated to delay queued items from immediate execution are no longer necessary.
Fabric and non-Fabric animations both look smooth after deleting this code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22807906
fbshipit-source-id: 9241fff84376f6aa9a35049cc40dfd6561effaa1
Summary:
Apparently it's possible for `!isEmpty()` to be true and `peek` to be non-null, but for `poll()` to be false. It doesn't really make sense to me, and I don't have a repro, but that's what logs are showing.
I suspect a teardown race condition or /maybe/ a Fabric/non-Fabric handoff race condition. Neither of those make a ton of sense, though.
The mitigation is fairly straightforward: we are just much more cautious with how we treat the queue and assume everything could be null.
This impacts Fabric and non-Fabric equally.
Changelog: [Internal]
Reviewed By: motiz88
Differential Revision: D22593924
fbshipit-source-id: 7748121951a64941fa6da2bd25ebf070be6dc89c
Summary:
There are potential race conditions in the old implementation that could result in operations being executed out-of-order when the user transitions between Fabric/non-Fabric screens.
Specifically, if the non-Fabric path queues up a batch of operations to execute on the next UI tick, it is possible that Fabric lifecycle events fire /before/ that next UI tick and synchronously execute a batch of animation operations on the UI thread, before their predecessors have executed.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22564633
fbshipit-source-id: 91c24547d5a682e61fc0c433302667330349a5f1
Summary:
See videos. In some cases when an `Animated.timing` animation was triggered, there are not necessarily any corresponding Fabric commits. This causes the animations to be queued up but never execute... until there's some Fabric commit at a distant point in the future.
We can't immediately flush all animation operations (see inline comments) but we also shouldn't wait forever. Thus, we wait 2 frames and then flush.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22490650
fbshipit-source-id: 1669bfed00f2a92b50f9558fc7ccaf71dc636980
Summary:
Expose `resolveCustomDirectEventName` from UIManager interface for Bridgeless Mode+NativeAnimatedModule.
We cannot remove the interface from the Non-Fabric UIManagerModule yet, as it would break downstream open-source dependencies, so I just marked it deprecated.
Note that this still doesn't totally fix issues with Bridgeless mode: generally I have to navigate to a Bridgeless surface, exit it, and then navigate back, and only on the 2nd navigation does everything seem to work properly...
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22483508
fbshipit-source-id: 685126e7e51aa5d0fd60ad5d4ecc44e8c6c3029d
Summary:
The previous assumption was that any animations would result in `addAnimatedEventToView` being called. That's not true in all cases, so sometimes we never subscribed to Fabric lifecycle events, preventing animations from working on some screens and for Venice.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22483509
fbshipit-source-id: c97576675902b4b9e1d4e659c7c1e24c5fe92946
Summary:
After animation has been finished using Native driver there is no final value passed from the native to JS side. This causes a bug from https://github.com/facebook/react-native/issues/28114.
This PR solves this problem in the same way as `react-native-reanimated` library. When detaching it is calling native side to get the last value from Animated node and stores it on the JS side.
Preserving animated value even if animation was using `useNativeDriver: true`
Fixes https://github.com/facebook/react-native/issues/28114
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[Internal] [Fixed] - Save native Animated node value on JS side in detach phase
Pull Request resolved: https://github.com/facebook/react-native/pull/28841
Test Plan: Unit tests for added getValue method passed. Green CI
Reviewed By: mdvacca
Differential Revision: D22211499
Pulled By: JoshuaGross
fbshipit-source-id: 9a3a98a9f9a8536fe2c8764f667cdabe1f6ba82a
Summary:
Switch between "Fabric" and "Non-Fabric" modes based on which types of native Views are being attached to animations. Don't allow non-Fabric to drive Fabric animations and vice-versa.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21985411
fbshipit-source-id: fb9bef1e38375b384430b4e0275e7b6d62eda7a4
Summary:
This is the second part of a rewrite of D15390384, which allows Animated timing to be driven by Paper or Fabric.
The intuition is: we don't care which one drives the animation. We will expect one or both of them to issue a callback that operations are about to be executed, and the first one wins. The blocks will only execute once, the second time will be a noop.
I don't think there's a 100% safe way of reimplementing Native Animated Module for Fabric/Venice (without a new API and implementing in C++) since it's inherently disconnected from the commit process and the tree. This gets us slightly closer to visual functionality, though.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21698192
fbshipit-source-id: c11d3cebd12cfc8acf4b63c87ccbe62cdbd8b672
Summary:
This is (part of) a rewrite of D15390384.
This implements the lifecycle interface only for Fabric to signal to NativeAnimatedModule when preOperations are about to run / operations are about to be dispatched.
We will likely want to remove this mechanism entirely and rewrite NativeAnimatedModule in C++ for Fabric/Venice, but for now, I'm not sure of a better solution to unblock.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21698193
fbshipit-source-id: a13d445073911fd63d896202a7a1bfbe1167038a
Summary:
This is the codemod portion of the parent diff.
I modified all call-sites to `ReactContext.getNativeModule` to do a null check on the returned NativeModule.
Changelog:
[Android][Fixed] - Check if NativeModules returned from CatalystInstanceImpl.getNativeModule are null before using them.
Reviewed By: JoshuaGross
Differential Revision: D21272028
fbshipit-source-id: 6bd16c6bf30605f2dfdf4c481352063712965342
Summary:
Fixed some typos in the comment.
## Changelog
[Internal] [Fixed] - Fixed typo in the comments
Pull Request resolved: https://github.com/facebook/react-native/pull/28269
Test Plan: Changes are only made in the comments, so test is not necessary.
Reviewed By: cpojer
Differential Revision: D20342637
Pulled By: shergin
fbshipit-source-id: f6e7dd538ee54c43e1570c35e1f8c4502054e328
Summary: Right now, animations don't work on Android in bridgeless mode because NativeAnimatedModule directly uses the UIManagerModule for various things. Previously, I added a bridgeless check to the module's `initialize()` method to avoid adding a listener on the UIManager; in this diff, I'm moving that check so that we also skip adding the lifecycle listener on the context in bridgeless mode, too. I'll remove this check once we come up with a replacement for the UIManagerModule API.
Reviewed By: JoshuaGross
Differential Revision: D19964171
fbshipit-source-id: 7c461f535e370b0e607c28905c505239cce0e157
Summary:
NativeAnimatedModule registers itself as a listener on UIManagerModule, which doesn't exist in bridgeless mode. We now have an API on ReactContext to detect if we're in bridgeless mode, so let's just bail out when that's the case (for now).
In the future, we'll need a replacement for this API on FabricUIManager (or somewhere).
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D19185762
fbshipit-source-id: 1cf53304ab58a5b985c8f4806544da51f09e8ba5
Summary:
All these NativeModules are now: (1) type-safe, (2) TurboModule-compatible.
**Note:** We still need to update `{Catalyst,Work,Fb4a}TurboModuleManagerDelegate` to understand these TurboModules. I'll most likely write up that diff and stack this one on top of it.
Changelog:
[Android][Added] - Make NativeModules TurboModule-compatible
Reviewed By: mdvacca
Differential Revision: D18888735
fbshipit-source-id: 34df64dc70e3f3a0a0303c049861205f9d3fd2ed
Summary:
This diff re-throw and logs exceptions in the animated module of RN Android
Changelog: internal
Reviewed By: JoshuaGross
Differential Revision: D18694124
fbshipit-source-id: bb4cb56dce99f09c56b0bc62733e8264f2df5a3f
Summary:
There are some cases where restoring default values on component unmount is not desirable. For example in react-native-screens we want to keep the native view displayed after react has unmounted them. Restoring default values causes an issue there because it will change props controlled my native animated back to their default value instead of keeping whatever value they had been animated to.
Restoring default values is only needed for updates anyway, where removing a prop controlled by native animated need to be reset to its default value since react no longer tracks its value.
This splits restoring default values and disconnecting from views in 2 separate native methods, this way we can restore default values only on component update and not on unmount. This takes care of being backwards compatible for JS running with the older native code.
## Changelog
[General] [Fixed] - NativeAnimated - Don't restore default values when components unmount
Pull Request resolved: https://github.com/facebook/react-native/pull/26978
Test Plan:
- Tested in an app using react-native-screens to make sure native views that are kept after their underlying component has been unmount don't change. Also tested in RNTester animated example.
- Tested that new JS works with old native code
Reviewed By: mmmulani
Differential Revision: D18197735
Pulled By: JoshuaGross
fbshipit-source-id: 20fa0f31a3edf1bc57ccb03df9d1486aba83edc4
Summary:
Simplify the API of `getReactApplicationContextIfActiveOrWarn`. We don't need to pass so much information into this method to collect good SoftExceptions.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18134400
fbshipit-source-id: 0a250ab0252a44121f3339a31506a0a6c4c7cd35
Summary:
In D18032458 we introduce `getReactApplicationContextIfActiveOrWarn`. In this diff, modules that access a JS or Native module through ReactApplicationContext need to check if the CatalystInstance is still alive before continuing.
Changelog: [Internal]
Reviewed By: furdei
Differential Revision: D18032788
fbshipit-source-id: 5152783afd0b93b8ce0970fe4a509ea71396a54a
Pieter De Baets
Genki Kondo
Pieter De Baets
Joshua Gross
Andrei Shikov
Andres Suarez
Ramanpreet Nara
Oleg Lokhvitsky
TMaszko
generatedunixname89002005287564
Jesse Katsumata
Emily Janzer
David Vacca
Janic Duplessis