Skip to content

Commit b6815c2

Browse files
Modify reanimated props updates (software-mansion#6330)
## Summary This PR changes the way `performOperations` and `ReanimatedCommitHook` are synchronized. The current implementations faces 2 problems: 1. Updates that come through `performOperations` after `pleaseSkipReanimatedCommit` is called in the commit hook will be deferred until the next commit. Usually it's fine since the next commit will be triggered by the next animation frame. But if there was a singular update scheduled through Reanimated, we might not see the change for a long time. This issue is more thoroughly explained in [this issue](software-mansion#6245). 2. In `ReanimatedCommitMarker` there is an assumption that there can be at most one commit happening on a given thread (i.e. there can't be nested commits). This isn't true, since there can be an event listener that commits some changes in a reaction to a native mount/unmount (which is a part of the commit function). [This exact scenario was observed in the New Expensify App with `react-native-keyboard-controller`](Expensify/App#44437 (comment)). At first I thought this is a mistake, but [this PR in RN](facebook/react-native@5f0435f) seems to allow for scenarios like that. Applied fixes: ad 1. Now instead of resetting the skip flag in reanimated after a transaction is mounted (via MountHook), we reset the flag whenever a non-empty batch is read in `performOperations`. This ensures that we don't make an unnecessary commit, but never skip any updates. ad 2. Now we mark reanimated commits through `ReanimatedCommitShadowNode`. This is a class derived from ShadowNode that allows us to modify the root nodes traits_ (and add our custom trait). This ensures that even if the root node gets cloned it will retain the information. We couldn't derive from `RootShadowNode` since it is a final class. ## Test plan
1 parent 08686b1 commit b6815c2

9 files changed

+63
-139
lines changed

Common/cpp/Fabric/PropsRegistry.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,9 @@ class PropsRegistry {
3939
#endif
4040
}
4141

42-
#if REACT_NATIVE_MINOR_VERSION >= 73
4342
void resetReanimatedSkipCommitFlag() {
4443
shouldReanimatedSkipCommit_ = false;
4544
}
46-
#endif
4745

4846
private:
4947
std::unordered_map<Tag, std::pair<ShadowNode::Shared, folly::dynamic>> map_;

Common/cpp/Fabric/ReanimatedCommitHook.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#include <vector>
66

77
#include "ReanimatedCommitHook.h"
8-
#include "ReanimatedCommitMarker.h"
8+
#include "ReanimatedCommitShadowNode.h"
99
#include "ShadowTreeCloner.h"
1010

1111
using namespace facebook::react;
@@ -31,9 +31,15 @@ RootShadowNode::Unshared ReanimatedCommitHook::shadowTreeWillCommit(
3131
#else
3232
RootShadowNode::Unshared const &newRootShadowNode) const noexcept {
3333
#endif
34-
if (ReanimatedCommitMarker::isReanimatedCommit()) {
34+
35+
auto reaShadowNode =
36+
std::reinterpret_pointer_cast<ReanimatedCommitShadowNode>(
37+
newRootShadowNode);
38+
39+
if (reaShadowNode->hasReanimatedCommitTrait()) {
3540
// ShadowTree commited by Reanimated, no need to apply updates from
3641
// PropsRegistry
42+
reaShadowNode->unsetReanimatedCommitTrait();
3743
return newRootShadowNode;
3844
}
3945

@@ -51,13 +57,14 @@ RootShadowNode::Unshared ReanimatedCommitHook::shadowTreeWillCommit(
5157
});
5258

5359
rootNode = cloneShadowTreeWithNewProps(*rootNode, propsMap);
54-
}
5560

56-
// If the commit comes from React Native then skip one commit from Reanimated
57-
// since the ShadowTree to be committed by Reanimated may not include the new
58-
// changes from React Native yet and all changes of animated props will be
59-
// applied in ReanimatedCommitHook by iterating over PropsRegistry.
60-
propsRegistry_->pleaseSkipReanimatedCommit();
61+
// If the commit comes from React Native then skip one commit from
62+
// Reanimated since the ShadowTree to be committed by Reanimated may not
63+
// include the new changes from React Native yet and all changes of animated
64+
// props will be applied in ReanimatedCommitHook by iterating over
65+
// PropsRegistry.
66+
propsRegistry_->pleaseSkipReanimatedCommit();
67+
}
6168

6269
return rootNode;
6370
}

Common/cpp/Fabric/ReanimatedCommitMarker.cpp

Lines changed: 0 additions & 26 deletions
This file was deleted.

Common/cpp/Fabric/ReanimatedCommitMarker.h

Lines changed: 0 additions & 23 deletions
This file was deleted.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#pragma once
2+
#ifdef RCT_NEW_ARCH_ENABLED
3+
4+
namespace reanimated {
5+
6+
// We use this trait to mark that a commit was created by Reanimated.
7+
// Traits are copied when nodes are cloned, so this information
8+
// won't be lost unless someone explicitly overrides it.
9+
// We need this information to skip unnecessary updates in
10+
// the commit hook.
11+
// Currently RN traits go up to 10, so hopefully
12+
// the arbitrarily chosen number 27 will be safe :)
13+
constexpr ShadowNodeTraits::Trait ReanimatedCommitTrait{1 << 27};
14+
15+
class ReanimatedCommitShadowNode : public ShadowNode {
16+
public:
17+
inline void setReanimatedCommitTrait() {
18+
traits_.set(ReanimatedCommitTrait);
19+
}
20+
inline void unsetReanimatedCommitTrait() {
21+
traits_.unset(ReanimatedCommitTrait);
22+
}
23+
inline bool hasReanimatedCommitTrait() {
24+
return traits_.check(ReanimatedCommitTrait);
25+
}
26+
};
27+
28+
} // namespace reanimated
29+
30+
#endif // RCT_NEW_ARCH_ENABLED

Common/cpp/Fabric/ReanimatedMountHook.cpp

Lines changed: 0 additions & 31 deletions
This file was deleted.

Common/cpp/Fabric/ReanimatedMountHook.h

Lines changed: 0 additions & 33 deletions
This file was deleted.

Common/cpp/NativeModules/NativeReanimatedModule.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
#ifdef RCT_NEW_ARCH_ENABLED
2020
#include <react/renderer/scheduler/Scheduler.h>
21-
#include "ReanimatedCommitMarker.h"
21+
#include "ReanimatedCommitShadowNode.h"
2222
#include "ShadowTreeCloner.h"
2323
#endif
2424

@@ -648,6 +648,10 @@ void NativeReanimatedModule::performOperations() {
648648
{
649649
auto lock = propsRegistry_->createLock();
650650

651+
if (copiedOperationsQueue.size() > 0) {
652+
propsRegistry_->resetReanimatedSkipCommitFlag();
653+
}
654+
651655
// remove recently unmounted ShadowNodes from PropsRegistry
652656
if (!tagsToRemove_.empty()) {
653657
for (auto tag : tagsToRemove_) {
@@ -713,10 +717,6 @@ void NativeReanimatedModule::performOperations() {
713717
const auto &shadowTreeRegistry = uiManager_->getShadowTreeRegistry();
714718

715719
shadowTreeRegistry.visit(surfaceId_, [&](ShadowTree const &shadowTree) {
716-
// Mark the commit as Reanimated commit so that we can distinguish it
717-
// in ReanimatedCommitHook.
718-
ReanimatedCommitMarker commitMarker;
719-
720720
shadowTree.commit(
721721
[&](RootShadowNode const &oldRootShadowNode)
722722
-> RootShadowNode::Unshared {
@@ -736,7 +736,19 @@ void NativeReanimatedModule::performOperations() {
736736
}
737737
#endif
738738
}
739-
return cloneShadowTreeWithNewProps(oldRootShadowNode, propsMap);
739+
740+
auto rootNode =
741+
cloneShadowTreeWithNewProps(oldRootShadowNode, propsMap);
742+
743+
// Mark the commit as Reanimated commit so that we can distinguish it
744+
// in ReanimatedCommitHook.
745+
746+
auto reaShadowNode =
747+
std::reinterpret_pointer_cast<ReanimatedCommitShadowNode>(
748+
rootNode);
749+
reaShadowNode->setReanimatedCommitTrait();
750+
751+
return rootNode;
740752
},
741753
{ /* .enableStateReconciliation = */
742754
false,
@@ -834,10 +846,6 @@ void NativeReanimatedModule::initializeFabric(
834846

835847
commitHook_ =
836848
std::make_shared<ReanimatedCommitHook>(propsRegistry_, uiManager_);
837-
#if REACT_NATIVE_MINOR_VERSION >= 73
838-
mountHook_ =
839-
std::make_shared<ReanimatedMountHook>(propsRegistry_, uiManager_);
840-
#endif
841849
}
842850

843851
void NativeReanimatedModule::initializeLayoutAnimations() {

Common/cpp/NativeModules/NativeReanimatedModule.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323
#include "LayoutAnimationsProxy.h"
2424
#include "PropsRegistry.h"
2525
#include "ReanimatedCommitHook.h"
26-
#if REACT_NATIVE_MINOR_VERSION >= 73
27-
#include "ReanimatedMountHook.h"
28-
#endif
2926
#endif
3027

3128
namespace reanimated {
@@ -221,9 +218,6 @@ class NativeReanimatedModule : public NativeReanimatedModuleSpec {
221218

222219
std::shared_ptr<PropsRegistry> propsRegistry_;
223220
std::shared_ptr<ReanimatedCommitHook> commitHook_;
224-
#if REACT_NATIVE_MINOR_VERSION >= 73
225-
std::shared_ptr<ReanimatedMountHook> mountHook_;
226-
#endif
227221

228222
std::vector<Tag> tagsToRemove_; // from `propsRegistry_`
229223
#else

0 commit comments

Comments
 (0)