Skip to content

Commit

Permalink
Modify reanimated props updates (#6330)
Browse files Browse the repository at this point in the history
## 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](#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
  • Loading branch information
bartlomiejbloniarz authored Jul 30, 2024
1 parent a9dc639 commit 071e513
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 139 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,9 @@ class PropsRegistry {
#endif
}

#if REACT_NATIVE_MINOR_VERSION >= 73
void resetReanimatedSkipCommitFlag() {
shouldReanimatedSkipCommit_ = false;
}
#endif

private:
std::unordered_map<Tag, std::pair<ShadowNode::Shared, folly::dynamic>> map_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <vector>

#include "ReanimatedCommitHook.h"
#include "ReanimatedCommitMarker.h"
#include "ReanimatedCommitShadowNode.h"
#include "ShadowTreeCloner.h"

using namespace facebook::react;
Expand All @@ -31,9 +31,15 @@ RootShadowNode::Unshared ReanimatedCommitHook::shadowTreeWillCommit(
#else
RootShadowNode::Unshared const &newRootShadowNode) const noexcept {
#endif
if (ReanimatedCommitMarker::isReanimatedCommit()) {

auto reaShadowNode =
std::reinterpret_pointer_cast<ReanimatedCommitShadowNode>(
newRootShadowNode);

if (reaShadowNode->hasReanimatedCommitTrait()) {
// ShadowTree commited by Reanimated, no need to apply updates from
// PropsRegistry
reaShadowNode->unsetReanimatedCommitTrait();
return newRootShadowNode;
}

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

rootNode = cloneShadowTreeWithNewProps(*rootNode, propsMap);
}

// If the commit comes from React Native then skip one commit from Reanimated
// since the ShadowTree to be committed by Reanimated may not include the new
// changes from React Native yet and all changes of animated props will be
// applied in ReanimatedCommitHook by iterating over PropsRegistry.
propsRegistry_->pleaseSkipReanimatedCommit();
// If the commit comes from React Native then skip one commit from
// Reanimated since the ShadowTree to be committed by Reanimated may not
// include the new changes from React Native yet and all changes of animated
// props will be applied in ReanimatedCommitHook by iterating over
// PropsRegistry.
propsRegistry_->pleaseSkipReanimatedCommit();
}

return rootNode;
}
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma once
#ifdef RCT_NEW_ARCH_ENABLED

namespace reanimated {

// We use this trait to mark that a commit was created by Reanimated.
// Traits are copied when nodes are cloned, so this information
// won't be lost unless someone explicitly overrides it.
// We need this information to skip unnecessary updates in
// the commit hook.
// Currently RN traits go up to 10, so hopefully
// the arbitrarily chosen number 27 will be safe :)
constexpr ShadowNodeTraits::Trait ReanimatedCommitTrait{1 << 27};

class ReanimatedCommitShadowNode : public ShadowNode {
public:
inline void setReanimatedCommitTrait() {
traits_.set(ReanimatedCommitTrait);
}
inline void unsetReanimatedCommitTrait() {
traits_.unset(ReanimatedCommitTrait);
}
inline bool hasReanimatedCommitTrait() {
return traits_.check(ReanimatedCommitTrait);
}
};

} // namespace reanimated

#endif // RCT_NEW_ARCH_ENABLED

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#ifdef RCT_NEW_ARCH_ENABLED
#include <react/renderer/scheduler/Scheduler.h>
#include "ReanimatedCommitMarker.h"
#include "ReanimatedCommitShadowNode.h"
#include "ShadowTreeCloner.h"
#endif

Expand Down Expand Up @@ -648,6 +648,10 @@ void NativeReanimatedModule::performOperations() {
{
auto lock = propsRegistry_->createLock();

if (copiedOperationsQueue.size() > 0) {
propsRegistry_->resetReanimatedSkipCommitFlag();
}

// remove recently unmounted ShadowNodes from PropsRegistry
if (!tagsToRemove_.empty()) {
for (auto tag : tagsToRemove_) {
Expand Down Expand Up @@ -713,10 +717,6 @@ void NativeReanimatedModule::performOperations() {
const auto &shadowTreeRegistry = uiManager_->getShadowTreeRegistry();

shadowTreeRegistry.visit(surfaceId_, [&](ShadowTree const &shadowTree) {
// Mark the commit as Reanimated commit so that we can distinguish it
// in ReanimatedCommitHook.
ReanimatedCommitMarker commitMarker;

shadowTree.commit(
[&](RootShadowNode const &oldRootShadowNode)
-> RootShadowNode::Unshared {
Expand All @@ -736,7 +736,19 @@ void NativeReanimatedModule::performOperations() {
}
#endif
}
return cloneShadowTreeWithNewProps(oldRootShadowNode, propsMap);

auto rootNode =
cloneShadowTreeWithNewProps(oldRootShadowNode, propsMap);

// Mark the commit as Reanimated commit so that we can distinguish it
// in ReanimatedCommitHook.

auto reaShadowNode =
std::reinterpret_pointer_cast<ReanimatedCommitShadowNode>(
rootNode);
reaShadowNode->setReanimatedCommitTrait();

return rootNode;
},
{ /* .enableStateReconciliation = */
false,
Expand Down Expand Up @@ -834,10 +846,6 @@ void NativeReanimatedModule::initializeFabric(

commitHook_ =
std::make_shared<ReanimatedCommitHook>(propsRegistry_, uiManager_);
#if REACT_NATIVE_MINOR_VERSION >= 73
mountHook_ =
std::make_shared<ReanimatedMountHook>(propsRegistry_, uiManager_);
#endif
}

void NativeReanimatedModule::initializeLayoutAnimations() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
#include "LayoutAnimationsProxy.h"
#include "PropsRegistry.h"
#include "ReanimatedCommitHook.h"
#if REACT_NATIVE_MINOR_VERSION >= 73
#include "ReanimatedMountHook.h"
#endif
#endif

namespace reanimated {
Expand Down Expand Up @@ -221,9 +218,6 @@ class NativeReanimatedModule : public NativeReanimatedModuleSpec {

std::shared_ptr<PropsRegistry> propsRegistry_;
std::shared_ptr<ReanimatedCommitHook> commitHook_;
#if REACT_NATIVE_MINOR_VERSION >= 73
std::shared_ptr<ReanimatedMountHook> mountHook_;
#endif

std::vector<Tag> tagsToRemove_; // from `propsRegistry_`
#else
Expand Down

0 comments on commit 071e513

Please sign in to comment.