Skip to content

Commit

Permalink
Better reanimated shadow tree commit detection (#4949)
Browse files Browse the repository at this point in the history
## Summary

Proposed solution is more bullet proof solution for detecting whether
currently committed shadow tree comes from reanimated.

Here are the main issues with current implementation:
- if there's another commit hook that is executed before `reanimated`
one, then `propsRegistry_->isLastReanimatedRoot(newRootShadowNode)` may
not work if the commit hook creates a copy of committed tree
- when `stateReconciliation` is enabled via commit options then new tree
root is created and again above detection does not work.

It uses concept of `thread_local` c++ variable which has instance for
each thread and RAII to ensure that the appropriate flag is set to
`true` on entering commit section and reset to `false` after commit from
reanimated on UI thread.

## Test plan

- enable state reconciliation for commit from `reanimated` and check
whether the code goes into `isReanimatedCommit` `if` body in
`ReanimatedCommitHook`

or

- create a custom commit hook and register it before
`ReanimatedCommitHook` and perform same check as above
  • Loading branch information
michalmaka committed Aug 21, 2023
1 parent 8101d9f commit e33d517
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 16 deletions.
12 changes: 0 additions & 12 deletions Common/cpp/Fabric/PropsRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ class PropsRegistry {

void remove(const Tag tag);

void setLastReanimatedRoot(
RootShadowNode::Shared const &lastReanimatedRoot) noexcept {
lastReanimatedRoot_.store(lastReanimatedRoot.get());
}

bool isLastReanimatedRoot(
RootShadowNode::Shared const &newRootShadowNode) const noexcept {
return newRootShadowNode.get() == lastReanimatedRoot_.load();
}

void pleaseSkipCommit() {
letMeIn_ = true;
}
Expand All @@ -48,8 +38,6 @@ class PropsRegistry {

mutable std::mutex mutex_; // Protects `map_`.

std::atomic<const RootShadowNode *> lastReanimatedRoot_;

std::atomic<bool> letMeIn_;
};

Expand Down
3 changes: 2 additions & 1 deletion Common/cpp/Fabric/ReanimatedCommitHook.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <react/renderer/core/ComponentDescriptor.h>

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

using namespace facebook::react;
Expand All @@ -24,7 +25,7 @@ RootShadowNode::Unshared ReanimatedCommitHook::shadowTreeWillCommit(
ShadowTree const &,
RootShadowNode::Shared const &,
RootShadowNode::Unshared const &newRootShadowNode) const noexcept {
if (propsRegistry_->isLastReanimatedRoot(newRootShadowNode)) {
if (ReanimatedCommitMarker::isReanimatedCommit()) {
// ShadowTree commited by Reanimated, no need to apply updates from
// PropsRegistry
return newRootShadowNode;
Expand Down
26 changes: 26 additions & 0 deletions Common/cpp/Fabric/ReanimatedCommitMarker.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#ifdef RCT_NEW_ARCH_ENABLED

#include "ReanimatedCommitMarker.h"

#include <react/debug/react_native_assert.h>

namespace reanimated {

thread_local bool ReanimatedCommitMarker::reanimatedCommitFlag_{false};

ReanimatedCommitMarker::ReanimatedCommitMarker() {
react_native_assert(reanimatedCommitFlag_ != true);
reanimatedCommitFlag_ = true;
}

ReanimatedCommitMarker::~ReanimatedCommitMarker() {
reanimatedCommitFlag_ = false;
}

bool ReanimatedCommitMarker::isReanimatedCommit() {
return reanimatedCommitFlag_;
}

} // namespace reanimated

#endif // RCT_NEW_ARCH_ENABLED
23 changes: 23 additions & 0 deletions Common/cpp/Fabric/ReanimatedCommitMarker.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#pragma once
#ifdef RCT_NEW_ARCH_ENABLED

namespace reanimated {

// This class is used to mark shadow tree commit as coming from Reanimated.
// During the life of this object, isReanimatedCommit() will return true, false
// otherwise. isReanimatedCommit() value change is restricted to the thread that
// created the object.
class ReanimatedCommitMarker {
public:
ReanimatedCommitMarker();
~ReanimatedCommitMarker();

static bool isReanimatedCommit();

private:
static thread_local bool reanimatedCommitFlag_;
};

} // namespace reanimated

#endif // RCT_NEW_ARCH_ENABLED
8 changes: 5 additions & 3 deletions Common/cpp/NativeModules/NativeReanimatedModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#ifdef RCT_NEW_ARCH_ENABLED
#include "FabricUtils.h"
#include "PropsRegistry.h"
#include "ReanimatedCommitMarker.h"
#include "ShadowTreeCloner.h"
#endif

Expand Down Expand Up @@ -621,6 +622,10 @@ 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) {
auto rootNode =
Expand All @@ -646,9 +651,6 @@ void NativeReanimatedModule::performOperations() {

auto newRoot = std::static_pointer_cast<RootShadowNode>(rootNode);

// skip ReanimatedCommitHook for this ShadowTree
propsRegistry_->setLastReanimatedRoot(newRoot);

return newRoot;
},
{/* default commit options */});
Expand Down

0 comments on commit e33d517

Please sign in to comment.