From 1e4d8d902daca8e524ba67fc3c1f4b77698c4d08 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Sat, 15 Aug 2020 19:18:36 -0700 Subject: [PATCH] Core/Differ: detect and optimize reparenting Summary: # Summary In previous diffs earlier in 2020, we made changes to detect and optimize reordering of views when the order of views changed underneath the same parent. However, until now we have ignored reparenting and there's evidence of issues because of that. Because Fabric flattens views more aggressively, reparenting is also marginally more likely to happen. This diff introduces a very general Reparenting detection. It will work with view flattening/unflattening, as well as tree grafting - subtrees moved to entirely different parts of the tree, not just a single parent disappearing or reappearing because of flattening/unflattening. There is also another consideration: previously, we were generating strictly too many Create+Delete operations that were redundant and could cause consistency issues, crashes, or bugs on platforms that do not handle that gracefully - especially since the ordering of the Create+Delete is not guaranteed (a reparented view could be created "first" and then the differ could later issue a "delete" for the same view). Intuition behind how it works: we know the cases where we can detect reparenting: it's when nodes are *not* matched up with another node from the other tree, and we're either trying to delete an entire subtree, or create an entire subtree. For perf reasons, we generate whatever set of operations comes first (say, we generate all the Delete and Remove instructions) and take note in the `ReparentingMetadata` data-structure that Delete and/or Remove have been performed for each tag (if ordering is different, we do the same for Create+Insert if those come first). Then if we later detect a corresponding subtree creation/deletion, we don't generate those mutations and we mark the previous mutations for deletion. This incurs some map lookup cost, but this is only wasteful for commits where a large tree is deleted and a large tree is created, without reparenting. We may be able to improve perf further for certain edge-cases in the future. # Why can't we solve this in JS? Two things: 1. We certainly can avoid reparenting situations in JS, but it's trickier than before because of Fabric's view flattening logic - product engineers would have to think much harder about how to prevent reparenting in the general case. 2. In the case of specific views like BottomSheet that may crash if they're reparented, the solution is to make sure that the BottomSheet and the first child of the BottomSheet is never memoized, so that lifecycle functions and render are called more often; and that in every render, the BottomSheet manually clones its child, so that when the Views are recreated, the child of the BottomSheet has a tag and is an entirely different instance. This is certainly possible to do but feels like an onerous requirement for product teams, and it could be challenging to track down every specific BottomSheet that is memoized and/or hoist them higher in the view hierarchy so they're not reparented as often. Reviewed By: shergin Differential Revision: D23123575 fbshipit-source-id: 2fa7e1f026f87b6f0c60cad469a3ba85cdc234de --- .../renderer/mounting/Differentiator.cpp | 417 ++++++++++++++- .../react/renderer/mounting/Differentiator.h | 3 +- .../renderer/mounting/MountingCoordinator.cpp | 10 +- .../renderer/mounting/MountingCoordinator.h | 5 +- .../react/renderer/mounting/ShadowTree.cpp | 11 +- .../react/renderer/mounting/ShadowTree.h | 7 +- .../renderer/mounting/ShadowViewMutation.h | 2 +- .../renderer/mounting/tests/MountingTest.cpp | 486 +++++++++++++++--- .../react/renderer/scheduler/Scheduler.cpp | 7 +- .../react/renderer/scheduler/Scheduler.h | 1 + 10 files changed, 852 insertions(+), 97 deletions(-) diff --git a/ReactCommon/react/renderer/mounting/Differentiator.cpp b/ReactCommon/react/renderer/mounting/Differentiator.cpp index fce2a721d1ad02..5d1d1f3601d5c6 100644 --- a/ReactCommon/react/renderer/mounting/Differentiator.cpp +++ b/ReactCommon/react/renderer/mounting/Differentiator.cpp @@ -14,6 +14,16 @@ #include #include "ShadowView.h" +// Uncomment this to enable verbose diffing logs, which can be useful for +// debugging. #define DEBUG_LOGS_DIFFER + +#ifdef DEBUG_LOGS_DIFFER +#include +#define DEBUG_LOGS(code) code +#else +#define DEBUG_LOGS(code) +#endif + namespace facebook { namespace react { @@ -149,6 +159,206 @@ class TinyMap final { int erasedAtFront_{0}; }; +struct OperationsOnTag final { + int shouldEraseOp = 0; + int opExists = 0; + + int removeInsertIndex = -1; + Tag parentTag = + -1; // the parent tag of Remove or Insert, whichever comes first + + ShadowNode const *oldNode; + ShadowNode const *newNode; +}; + +class ReparentingMetadata final { + public: + ReparentingMetadata(bool enabled) : enabled_(enabled) {} + + /** + * Returns a triple: (ShouldRemove, ShouldDelete, ShadowNode pointer if + * updating) + * + * @param mutation + * @return + */ + std::tuple shouldRemoveDeleteUpdate( + Tag parentTag, + ShadowNode const *shadowNode, + int index) { + if (!enabled_) { + return std::tuple(true, true, nullptr); + } + + Tag tag = shadowNode->getTag(); + + auto it = tagsToOperations_.find(tag); + if (it == tagsToOperations_.end()) { + auto tagOperations = OperationsOnTag{}; + tagOperations.removeInsertIndex = index; + tagOperations.parentTag = parentTag; + tagOperations.opExists |= + ShadowViewMutation::Type::Remove | ShadowViewMutation::Type::Delete; + tagOperations.oldNode = shadowNode; + tagsToOperations_[tag] = tagOperations; + return std::tuple(true, true, nullptr); + } + + assert(it->second.shouldEraseOp == 0); + bool shouldRemove = + !((it->second.opExists & ShadowViewMutation::Type::Insert) && + it->second.removeInsertIndex == index && + it->second.parentTag == parentTag); + it->second.shouldEraseOp |= + (it->second.opExists & ShadowViewMutation::Type::Create); + it->second.shouldEraseOp |= shouldRemove + ? 0 + : (it->second.opExists & ShadowViewMutation::Type::Insert); + + if (it->second.shouldEraseOp != 0) { + reparentingOperations_++; + } + + // TODO: At this point we are *done* with this record in the map besides + // postprocessing, and we know we've reparented. There is a potential + // optimization here. + + return std::tuple( + shouldRemove, false, it->second.newNode); + } + + /** + * Returns a triple: (ShouldCreate, ShouldInsert, ShadowNode pointer if + * updating) + * + * @param mutation + * @return + */ + std::tuple shouldCreateInsertUpdate( + Tag parentTag, + ShadowNode const *shadowNode, + int index) { + if (!enabled_) { + return std::tuple(true, true, nullptr); + } + + Tag tag = shadowNode->getTag(); + + auto it = tagsToOperations_.find(tag); + if (it == tagsToOperations_.end()) { + auto tagOperations = OperationsOnTag{}; + tagOperations.removeInsertIndex = index; + tagOperations.opExists |= + ShadowViewMutation::Type::Create | ShadowViewMutation::Type::Insert; + tagOperations.newNode = shadowNode; + tagsToOperations_[tag] = tagOperations; + return std::tuple(true, true, nullptr); + } + + assert(it->second.shouldEraseOp == 0); + bool shouldInsert = + !((it->second.opExists & ShadowViewMutation::Type::Remove) && + it->second.removeInsertIndex == index && + it->second.parentTag == parentTag); + it->second.shouldEraseOp |= + (it->second.opExists & ShadowViewMutation::Type::Delete); + it->second.shouldEraseOp |= shouldInsert + ? 0 + : (it->second.opExists & ShadowViewMutation::Type::Remove); + + if (it->second.shouldEraseOp != 0) { + reparentingOperations_++; + } + + // TODO: At this point we are *done* with this record in the map besides + // postprocessing, and we know we've reparented. There is a potential + // optimization here. + + return std::tuple( + shouldInsert, false, it->second.oldNode); + } + + /** + * Returns a pair: (ShouldCreate, ShadowNode pointer if updating) + * This is called in a case where a node has *already* been inserted. + * + * @param mutation + * @return + */ + std::pair shouldCreateUpdate( + ShadowNode const *shadowNode) { + if (!enabled_) { + return std::pair(true, nullptr); + } + + Tag tag = shadowNode->getTag(); + + auto it = tagsToOperations_.find(tag); + assert(it != tagsToOperations_.end()); + + if (it->second.opExists & ShadowViewMutation::Type::Delete) { + reparentingOperations_++; + it->second.shouldEraseOp |= ShadowViewMutation::Type::Delete; + it->second.newNode = shadowNode; // Is this necessary? + return std::pair(false, it->second.oldNode); + } + + it->second.opExists |= ShadowViewMutation::Type::Create; + return std::pair(true, nullptr); + } + + /** + * Insertion is happening due to reordering and likely cannot be canceled. + * + * @param shadowNode + * @param index + */ + void markInserted(Tag parentTag, ShadowNode const *shadowNode, int index) { + if (!enabled_) { + return; + } + + Tag tag = shadowNode->getTag(); + + auto it = tagsToOperations_.find(tag); + if (it == tagsToOperations_.end()) { + auto tagOperations = OperationsOnTag{}; + tagOperations.removeInsertIndex = index; + tagOperations.parentTag = parentTag; + it->second.opExists |= ShadowViewMutation::Type::Insert; + tagsToOperations_[tag] = tagOperations; + return; + } + + // Element was moved from somewhere else in the hierarchy and inserted + // in a new position - we can't cancel this operation. + it->second.opExists |= ShadowViewMutation::Type::Insert; + } + + /** + * Use this to prepare for iterating over records for ShadowViewMutation + * removal. This removes unnecessary information from the map. + */ + void removeUselessRecords() { + if (!enabled_) { + return; + } + + for (auto it = tagsToOperations_.begin(); it != tagsToOperations_.end();) { + OperationsOnTag &op = it->second; + if (op.shouldEraseOp == 0) { + it = tagsToOperations_.erase(it); + } else { + it++; + } + } + } + + bool enabled_{false}; + int reparentingOperations_ = 0; + std::map tagsToOperations_; +}; + /* * Sorting comparator for `reorderInPlaceIfNeeded`. */ @@ -266,6 +476,7 @@ static_assert( static void calculateShadowViewMutations( ShadowViewMutation::List &mutations, + ReparentingMetadata &reparentingMetadata, ShadowView const &parentShadowView, ShadowViewNodePair::List &&oldChildPairs, ShadowViewNodePair::List &&newChildPairs) { @@ -295,10 +506,22 @@ static void calculateShadowViewMutations( auto const &newChildPair = newChildPairs[index]; if (oldChildPair.shadowView.tag != newChildPair.shadowView.tag) { + DEBUG_LOGS({ + LOG(ERROR) << "Differ Branch 1.1: Tags Different: [" + << oldChildPair.shadowView.tag << "] [" + << newChildPair.shadowView.tag << "]"; + }); + // Totally different nodes, updating is impossible. break; } + DEBUG_LOGS({ + LOG(ERROR) << "Differ Branch 1.2: Same tags, update and recurse: [" + << oldChildPair.shadowView.tag << "] [" + << newChildPair.shadowView.tag << "]"; + }); + if (oldChildPair.shadowView != newChildPair.shadowView) { updateMutations.push_back(ShadowViewMutation::UpdateMutation( parentShadowView, @@ -314,6 +537,7 @@ static void calculateShadowViewMutations( calculateShadowViewMutations( *(newGrandChildPairs.size() ? &downwardMutations : &destructiveDownwardMutations), + reparentingMetadata, oldChildPair.shadowView, std::move(oldGrandChildPairs), std::move(newGrandChildPairs)); @@ -327,15 +551,40 @@ static void calculateShadowViewMutations( for (; index < oldChildPairs.size(); index++) { auto const &oldChildPair = oldChildPairs[index]; - deleteMutations.push_back( - ShadowViewMutation::DeleteMutation(oldChildPair.shadowView)); - removeMutations.push_back(ShadowViewMutation::RemoveMutation( - parentShadowView, oldChildPair.shadowView, index)); + DEBUG_LOGS({ + LOG(ERROR) + << "Differ Branch 2: Deleting Tag/Tree (may be reparented): [" + << oldChildPair.shadowView.tag << "]"; + }); + + bool shouldRemove; + bool shouldDelete; + ShadowNode const *newTreeNode; + std::tie(shouldRemove, shouldDelete, newTreeNode) = + reparentingMetadata.shouldRemoveDeleteUpdate( + parentShadowView.tag, oldChildPair.shadowNode, index); + + if (shouldDelete) { + deleteMutations.push_back( + ShadowViewMutation::DeleteMutation(oldChildPair.shadowView)); + } + if (shouldRemove) { + removeMutations.push_back(ShadowViewMutation::RemoveMutation( + parentShadowView, oldChildPair.shadowView, index)); + } + if (newTreeNode != nullptr) { + ShadowView newTreeNodeView = ShadowView(*newTreeNode); + if (newTreeNodeView != oldChildPair.shadowView) { + updateMutations.push_back(ShadowViewMutation::UpdateMutation( + parentShadowView, oldChildPair.shadowView, newTreeNodeView, -1)); + } + } // We also have to call the algorithm recursively to clean up the entire // subtree starting from the removed view. calculateShadowViewMutations( destructiveDownwardMutations, + reparentingMetadata, oldChildPair.shadowView, sliceChildShadowNodeViewPairs(*oldChildPair.shadowNode), {}); @@ -346,13 +595,38 @@ static void calculateShadowViewMutations( for (; index < newChildPairs.size(); index++) { auto const &newChildPair = newChildPairs[index]; - insertMutations.push_back(ShadowViewMutation::InsertMutation( - parentShadowView, newChildPair.shadowView, index)); - createMutations.push_back( - ShadowViewMutation::CreateMutation(newChildPair.shadowView)); + DEBUG_LOGS({ + LOG(ERROR) + << "Differ Branch 3: Creating Tag/Tree (may be reparented): [" + << newChildPair.shadowView.tag << "]"; + }); + + bool shouldInsert; + bool shouldCreate; + ShadowNode const *oldTreeNode; + std::tie(shouldInsert, shouldCreate, oldTreeNode) = + reparentingMetadata.shouldCreateInsertUpdate( + parentShadowView.tag, newChildPair.shadowNode, index); + + if (shouldInsert) { + insertMutations.push_back(ShadowViewMutation::InsertMutation( + parentShadowView, newChildPair.shadowView, index)); + } + if (shouldCreate) { + createMutations.push_back( + ShadowViewMutation::CreateMutation(newChildPair.shadowView)); + } + if (oldTreeNode != nullptr) { + ShadowView oldTreeNodeView = ShadowView(*oldTreeNode); + if (oldTreeNodeView != newChildPair.shadowView) { + updateMutations.push_back(ShadowViewMutation::UpdateMutation( + parentShadowView, oldTreeNodeView, newChildPair.shadowView, -1)); + } + } calculateShadowViewMutations( downwardMutations, + reparentingMetadata, newChildPair.shadowView, {}, sliceChildShadowNodeViewPairs(*newChildPair.shadowNode)); @@ -388,6 +662,13 @@ static void calculateShadowViewMutations( int oldTag = oldChildPair.shadowView.tag; if (newTag == oldTag) { + DEBUG_LOGS({ + LOG(ERROR) << "Differ Branch 5: Matched Tags at indices: " + << oldIndex << " " << newIndex << ": [" + << oldChildPair.shadowView.tag << "][" + << newChildPair.shadowView.tag << "]"; + }); + // Generate Update instructions if (oldChildPair.shadowView != newChildPair.shadowView) { updateMutations.push_back(ShadowViewMutation::UpdateMutation( @@ -411,6 +692,7 @@ static void calculateShadowViewMutations( calculateShadowViewMutations( *(newGrandChildPairs.size() ? &downwardMutations : &destructiveDownwardMutations), + reparentingMetadata, oldChildPair.shadowView, std::move(oldGrandChildPairs), std::move(newGrandChildPairs)); @@ -430,6 +712,12 @@ static void calculateShadowViewMutations( // remove the node from its old position now. auto const insertedIt = newInsertedPairs.find(oldTag); if (insertedIt != newInsertedPairs.end()) { + DEBUG_LOGS({ + LOG(ERROR) + << "Differ Branch 6: Removing tag that was already inserted: " + << oldIndex << ": [" << oldChildPair.shadowView.tag << "]"; + }); + removeMutations.push_back(ShadowViewMutation::RemoveMutation( parentShadowView, oldChildPair.shadowView, oldIndex)); @@ -452,6 +740,7 @@ static void calculateShadowViewMutations( calculateShadowViewMutations( *(newGrandChildPairs.size() ? &downwardMutations : &destructiveDownwardMutations), + reparentingMetadata, oldChildPair.shadowView, std::move(oldGrandChildPairs), std::move(newGrandChildPairs)); @@ -466,15 +755,45 @@ static void calculateShadowViewMutations( // generate remove+delete for this node and its subtree. auto const newIt = newRemainingPairs.find(oldTag); if (newIt == newRemainingPairs.end()) { + DEBUG_LOGS({ + LOG(ERROR) + << "Differ Branch 8: Removing tag/tree that was not reinserted (may be reparented): " + << oldIndex << ": [" << oldChildPair.shadowView.tag << "]"; + }); + + bool shouldRemove; + bool shouldDelete; + ShadowNode const *newTreeNode; + // Indices and parentTag don't matter here because we will always be + // executing this Remove operation, since it's in the context of + // reordering of views and the View was not already in this hierarchy. + std::tie(shouldRemove, shouldDelete, newTreeNode) = + reparentingMetadata.shouldRemoveDeleteUpdate( + -1, oldChildPair.shadowNode, -1); + removeMutations.push_back(ShadowViewMutation::RemoveMutation( parentShadowView, oldChildPair.shadowView, oldIndex)); - deleteMutations.push_back( - ShadowViewMutation::DeleteMutation(oldChildPair.shadowView)); + + if (shouldDelete) { + deleteMutations.push_back( + ShadowViewMutation::DeleteMutation(oldChildPair.shadowView)); + } + if (newTreeNode != nullptr) { + ShadowView newTreeNodeView = ShadowView(*newTreeNode); + if (newTreeNodeView != oldChildPair.shadowView) { + updateMutations.push_back(ShadowViewMutation::UpdateMutation( + parentShadowView, + oldChildPair.shadowView, + newTreeNodeView, + -1)); + } + } // We also have to call the algorithm recursively to clean up the // entire subtree starting from the removed view. calculateShadowViewMutations( destructiveDownwardMutations, + reparentingMetadata, oldChildPair.shadowView, sliceChildShadowNodeViewPairs(*oldChildPair.shadowNode), {}); @@ -488,6 +807,13 @@ static void calculateShadowViewMutations( // inserted or matched yet. We're not sure yet if the new node is in the // old list - generate an insert instruction for the new node. auto const &newChildPair = newChildPairs[newIndex]; + DEBUG_LOGS({ + LOG(ERROR) + << "Differ Branch 9: Inserting tag/tree that was not yet removed from hierarchy (may be reparented): " + << newIndex << ": [" << newChildPair.shadowView.tag << "]"; + }); + reparentingMetadata.markInserted( + parentShadowView.tag, newChildPair.shadowNode, newIndex); insertMutations.push_back(ShadowViewMutation::InsertMutation( parentShadowView, newChildPair.shadowView, newIndex)); newInsertedPairs.insert({newChildPair.shadowView.tag, &newChildPair}); @@ -506,11 +832,34 @@ static void calculateShadowViewMutations( } auto const &newChildPair = *it->second; - createMutations.push_back( - ShadowViewMutation::CreateMutation(newChildPair.shadowView)); + + DEBUG_LOGS({ + LOG(ERROR) + << "Differ Branch 9: Inserting tag/tree that was not yet removed from hierarchy (may be reparented): " + << newIndex << ": [" << newChildPair.shadowView.tag << "]"; + }); + + bool shouldCreate; + ShadowNode const *updateNode; + std::tie(shouldCreate, updateNode) = + reparentingMetadata.shouldCreateUpdate(newChildPair.shadowNode); + + if (shouldCreate) { + createMutations.push_back( + ShadowViewMutation::CreateMutation(newChildPair.shadowView)); + } + + if (updateNode != nullptr) { + ShadowView updateNodeView = ShadowView(*updateNode); + if (updateNodeView != newChildPair.shadowView) { + updateMutations.push_back(ShadowViewMutation::UpdateMutation( + parentShadowView, updateNodeView, newChildPair.shadowView, -1)); + } + } calculateShadowViewMutations( downwardMutations, + reparentingMetadata, newChildPair.shadowView, {}, sliceChildShadowNodeViewPairs(*newChildPair.shadowNode)); @@ -550,7 +899,8 @@ static void calculateShadowViewMutations( ShadowViewMutation::List calculateShadowViewMutations( ShadowNode const &oldRootShadowNode, - ShadowNode const &newRootShadowNode) { + ShadowNode const &newRootShadowNode, + bool enableReparentingDetection) { SystraceSection s("calculateShadowViewMutations"); // Root shadow nodes must be belong the same family. @@ -559,6 +909,8 @@ ShadowViewMutation::List calculateShadowViewMutations( auto mutations = ShadowViewMutation::List{}; mutations.reserve(256); + auto reparentingMetadata = ReparentingMetadata(enableReparentingDetection); + auto oldRootShadowView = ShadowView(oldRootShadowNode); auto newRootShadowView = ShadowView(newRootShadowNode); @@ -569,10 +921,49 @@ ShadowViewMutation::List calculateShadowViewMutations( calculateShadowViewMutations( mutations, + reparentingMetadata, ShadowView(oldRootShadowNode), sliceChildShadowNodeViewPairs(oldRootShadowNode), sliceChildShadowNodeViewPairs(newRootShadowNode)); + // Remove instructions obviated by reparenting + if (reparentingMetadata.reparentingOperations_ > 0 && + enableReparentingDetection) { + reparentingMetadata.removeUselessRecords(); + + mutations.erase( + std::remove_if( + mutations.begin(), + mutations.end(), + [&](ShadowViewMutation &mutation) { + if (reparentingMetadata.reparentingOperations_ == 0) { + return false; + } + + ShadowViewMutation::Type type = mutation.type; + Tag tag = type == ShadowViewMutation::Type::Insert || + type == ShadowViewMutation::Type::Create + ? mutation.newChildShadowView.tag + : mutation.oldChildShadowView.tag; + auto it = reparentingMetadata.tagsToOperations_.find(tag); + if (it != reparentingMetadata.tagsToOperations_.end()) { + bool shouldDelete = it->second.shouldEraseOp & type; + it->second.shouldEraseOp &= ~type; + + // We've done everything we need to with this record; delete it. + if (it->second.shouldEraseOp == 0) { + reparentingMetadata.tagsToOperations_.erase(it); + reparentingMetadata.reparentingOperations_--; + } + + return shouldDelete; + } + + return false; + }), + mutations.end()); + } + return mutations; } diff --git a/ReactCommon/react/renderer/mounting/Differentiator.h b/ReactCommon/react/renderer/mounting/Differentiator.h index 3befbd72b061c1..c68809ddac8c4f 100644 --- a/ReactCommon/react/renderer/mounting/Differentiator.h +++ b/ReactCommon/react/renderer/mounting/Differentiator.h @@ -22,7 +22,8 @@ enum class DifferentiatorMode { Classic, OptimizedMoves }; */ ShadowViewMutationList calculateShadowViewMutations( ShadowNode const &oldRootShadowNode, - ShadowNode const &newRootShadowNode); + ShadowNode const &newRootShadowNode, + bool enableReparentingDetection = false); /* * Generates a list of `ShadowViewNodePair`s that represents a layer of a diff --git a/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index 05bfa4adbf53b9..be48eee1dfe1c0 100644 --- a/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -21,11 +21,13 @@ namespace react { MountingCoordinator::MountingCoordinator( ShadowTreeRevision baseRevision, - std::weak_ptr delegate) + std::weak_ptr delegate, + bool enableReparentingDetection) : surfaceId_(baseRevision.getRootShadowNode().getSurfaceId()), baseRevision_(baseRevision), mountingOverrideDelegate_(delegate), - telemetryController_(*this) { + telemetryController_(*this), + enableReparentingDetection_(enableReparentingDetection) { #ifdef RN_SHADOW_TREE_INTROSPECTION stubViewTree_ = stubViewTreeFromShadowNode(baseRevision_.getRootShadowNode()); #endif @@ -139,7 +141,9 @@ better::optional MountingCoordinator::pullTransaction() telemetry.willDiff(); if (lastRevision_.hasValue()) { diffMutations = calculateShadowViewMutations( - baseRevision_.getRootShadowNode(), lastRevision_->getRootShadowNode()); + baseRevision_.getRootShadowNode(), + lastRevision_->getRootShadowNode(), + enableReparentingDetection_); } telemetry.didDiff(); diff --git a/ReactCommon/react/renderer/mounting/MountingCoordinator.h b/ReactCommon/react/renderer/mounting/MountingCoordinator.h index 88d2bf19d3f5cf..60884ee88bd304 100644 --- a/ReactCommon/react/renderer/mounting/MountingCoordinator.h +++ b/ReactCommon/react/renderer/mounting/MountingCoordinator.h @@ -41,7 +41,8 @@ class MountingCoordinator final { */ MountingCoordinator( ShadowTreeRevision baseRevision, - std::weak_ptr delegate); + std::weak_ptr delegate, + bool enableReparentingDetection = false); /* * Returns the id of the surface that the coordinator belongs to. @@ -109,6 +110,8 @@ class MountingCoordinator final { TelemetryController telemetryController_; + bool enableReparentingDetection_{false}; // temporary + #ifdef RN_SHADOW_TREE_INTROSPECTION void validateTransactionAgainstStubViewTree( ShadowViewMutationList const &mutations, diff --git a/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 4be53bf4252681..a0b5b2dc3c0644 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -222,8 +222,11 @@ ShadowTree::ShadowTree( LayoutContext const &layoutContext, RootComponentDescriptor const &rootComponentDescriptor, ShadowTreeDelegate const &delegate, - std::weak_ptr mountingOverrideDelegate) - : surfaceId_(surfaceId), delegate_(delegate) { + std::weak_ptr mountingOverrideDelegate, + bool enableReparentingDetection) + : surfaceId_(surfaceId), + delegate_(delegate), + enableReparentingDetection_(enableReparentingDetection) { const auto noopEventEmitter = std::make_shared( nullptr, -1, std::shared_ptr()); @@ -241,7 +244,9 @@ ShadowTree::ShadowTree( family)); mountingCoordinator_ = std::make_shared( - ShadowTreeRevision{rootShadowNode_, 0, {}}, mountingOverrideDelegate); + ShadowTreeRevision{rootShadowNode_, 0, {}}, + mountingOverrideDelegate, + enableReparentingDetection); } ShadowTree::~ShadowTree() { diff --git a/ReactCommon/react/renderer/mounting/ShadowTree.h b/ReactCommon/react/renderer/mounting/ShadowTree.h index 6bbe1631cf7a0b..70155de275f11d 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTree.h +++ b/ReactCommon/react/renderer/mounting/ShadowTree.h @@ -40,7 +40,8 @@ class ShadowTree final { LayoutContext const &layoutContext, RootComponentDescriptor const &rootComponentDescriptor, ShadowTreeDelegate const &delegate, - std::weak_ptr mountingOverrideDelegate); + std::weak_ptr mountingOverrideDelegate, + bool enableReparentingDetection = false); ~ShadowTree(); @@ -87,6 +88,9 @@ class ShadowTree final { void setEnableNewStateReconciliation(bool value) { enableNewStateReconciliation_ = value; } + void setEnableReparentingDetection(bool value) { + enableReparentingDetection_ = value; + } private: RootShadowNode::Unshared cloneRootShadowNode( @@ -106,6 +110,7 @@ class ShadowTree final { 0}; // Protected by `commitMutex_`. MountingCoordinator::Shared mountingCoordinator_; bool enableNewStateReconciliation_{false}; + bool enableReparentingDetection_{false}; }; } // namespace react diff --git a/ReactCommon/react/renderer/mounting/ShadowViewMutation.h b/ReactCommon/react/renderer/mounting/ShadowViewMutation.h index 65b52beff39f27..07ee6bcaf7e482 100644 --- a/ReactCommon/react/renderer/mounting/ShadowViewMutation.h +++ b/ReactCommon/react/renderer/mounting/ShadowViewMutation.h @@ -62,7 +62,7 @@ struct ShadowViewMutation final { #pragma mark - Type - enum Type { Create, Delete, Insert, Remove, Update }; + enum Type { Create = 1, Delete = 2, Insert = 4, Remove = 8, Update = 16 }; #pragma mark - Fields diff --git a/ReactCommon/react/renderer/mounting/tests/MountingTest.cpp b/ReactCommon/react/renderer/mounting/tests/MountingTest.cpp index 275daa6b60f3ed..735de5884a6b64 100644 --- a/ReactCommon/react/renderer/mounting/tests/MountingTest.cpp +++ b/ReactCommon/react/renderer/mounting/tests/MountingTest.cpp @@ -20,32 +20,45 @@ namespace facebook { namespace react { -static ShadowNode::Shared makeNode( - ComponentDescriptor const &componentDescriptor, - int tag, - ShadowNode::ListOfShared children) { - auto props = generateDefaultProps(componentDescriptor); - - // Make sure node is layoutable by giving it dimensions and making it - // accessible This is an implementation detail and subject to change. +static SharedViewProps nonFlattenedDefaultProps( + ComponentDescriptor const &componentDescriptor) { folly::dynamic dynamic = folly::dynamic::object(); dynamic["position"] = "absolute"; dynamic["top"] = 0; dynamic["left"] = 0; dynamic["width"] = 100; dynamic["height"] = 100; - dynamic["nativeId"] = tag; + dynamic["nativeId"] = "NativeId"; dynamic["accessible"] = true; - auto newProps = componentDescriptor.cloneProps(props, RawProps(dynamic)); + return std::static_pointer_cast( + componentDescriptor.cloneProps(nullptr, RawProps{dynamic})); +} + +static ShadowNode::Shared makeNode( + ComponentDescriptor const &componentDescriptor, + int tag, + ShadowNode::ListOfShared children, + bool flattened = false) { + auto props = flattened ? generateDefaultProps(componentDescriptor) + : nonFlattenedDefaultProps(componentDescriptor); return componentDescriptor.createShadowNode( - ShadowNodeFragment{newProps, + ShadowNodeFragment{props, std::make_shared(children)}, componentDescriptor.createFamily({tag, SurfaceId(1), nullptr}, nullptr)); } -TEST(MountingTest, testMinimalInstructionGeneration) { +/** + * Test reordering of views with the same parent: + * + * For instance: + * A -> [B,C,D] ==> A -> [D,B,C] + * + * In the V1 of diffing this would produce 3 removes and 3 inserts, but with + * some cleverness we can reduce this to 1 remove and 1 insert. + */ +TEST(MountingTest, testReorderingInstructionGeneration) { auto eventDispatcher = EventDispatcher::Shared{}; auto contextContainer = std::make_shared(); auto componentDescriptorParameters = @@ -161,7 +174,7 @@ TEST(MountingTest, testMinimalInstructionGeneration) { std::make_shared( SharedShadowNodeList{shadowNodeV7})})); - // Layout and diff + // Layout std::vector affectedLayoutableNodesV1{}; affectedLayoutableNodesV1.reserve(1024); std::const_pointer_cast(rootNodeV1) @@ -227,7 +240,8 @@ TEST(MountingTest, testMinimalInstructionGeneration) { }*/ // Calculating mutations. - auto mutations1 = calculateShadowViewMutations(*rootNodeV1, *rootNodeV2); + auto mutations1 = + calculateShadowViewMutations(*rootNodeV1, *rootNodeV2, false); // The order and exact mutation instructions here may change at any time. // This test just ensures that any changes are intentional. @@ -235,15 +249,16 @@ TEST(MountingTest, testMinimalInstructionGeneration) { // produces a single "Insert" instruction, and no remove/insert (move) // operations. All these nodes are laid out with absolute positioning, so // moving them around does not change layout. - assert(mutations1.size() == 2); - assert(mutations1[0].type == ShadowViewMutation::Create); - assert(mutations1[0].newChildShadowView.tag == 100); - assert(mutations1[1].type == ShadowViewMutation::Insert); - assert(mutations1[1].newChildShadowView.tag == 100); - assert(mutations1[1].index == 0); + EXPECT_TRUE(mutations1.size() == 2); + EXPECT_TRUE(mutations1[0].type == ShadowViewMutation::Create); + EXPECT_TRUE(mutations1[0].newChildShadowView.tag == 100); + EXPECT_TRUE(mutations1[1].type == ShadowViewMutation::Insert); + EXPECT_TRUE(mutations1[1].newChildShadowView.tag == 100); + EXPECT_TRUE(mutations1[1].index == 0); // Calculating mutations. - auto mutations2 = calculateShadowViewMutations(*rootNodeV2, *rootNodeV3); + auto mutations2 = + calculateShadowViewMutations(*rootNodeV2, *rootNodeV3, false); // The order and exact mutation instructions here may change at any time. // This test just ensures that any changes are intentional. @@ -251,15 +266,17 @@ TEST(MountingTest, testMinimalInstructionGeneration) { // produces a single remove (and delete) instruction, and no remove/insert // (move) operations. All these nodes are laid out with absolute positioning, // so moving them around does not change layout. - assert(mutations2.size() == 2); - assert(mutations2[0].type == ShadowViewMutation::Remove); - assert(mutations2[0].oldChildShadowView.tag == 100); - assert(mutations2[0].index == 0); - assert(mutations2[1].type == ShadowViewMutation::Delete); - assert(mutations2[1].oldChildShadowView.tag == 100); + EXPECT_TRUE(mutations2.size() == 2); + EXPECT_TRUE(mutations2[0].type == ShadowViewMutation::Remove); + EXPECT_TRUE(mutations2[0].oldChildShadowView.tag == 100); + EXPECT_TRUE(mutations2[0].index == 0); + EXPECT_TRUE(mutations2[1].type == ShadowViewMutation::Delete); + EXPECT_TRUE(mutations2[1].oldChildShadowView.tag == 100); // Calculating mutations. - auto mutations3 = calculateShadowViewMutations(*rootNodeV3, *rootNodeV4); + auto mutations3 = + calculateShadowViewMutations(*rootNodeV3, *rootNodeV4, false); + LOG(ERROR) << "Num mutations IN OLD TEST mutations3: " << mutations3.size(); // The order and exact mutation instructions here may change at any time. // This test just ensures that any changes are intentional. @@ -267,20 +284,21 @@ TEST(MountingTest, testMinimalInstructionGeneration) { // produces a single remove (and delete) instruction, and no remove/insert // (move) operations; and that simultaneously, we can insert a node at the // end. - assert(mutations3.size() == 4); - assert(mutations3[0].type == ShadowViewMutation::Remove); - assert(mutations3[0].oldChildShadowView.tag == 102); - assert(mutations3[0].index == 1); - assert(mutations3[1].type == ShadowViewMutation::Delete); - assert(mutations3[1].oldChildShadowView.tag == 102); - assert(mutations3[2].type == ShadowViewMutation::Create); - assert(mutations3[2].newChildShadowView.tag == 104); - assert(mutations3[3].type == ShadowViewMutation::Insert); - assert(mutations3[3].newChildShadowView.tag == 104); - assert(mutations3[3].index == 2); + EXPECT_TRUE(mutations3.size() == 4); + EXPECT_TRUE(mutations3[0].type == ShadowViewMutation::Remove); + EXPECT_TRUE(mutations3[0].oldChildShadowView.tag == 102); + EXPECT_TRUE(mutations3[0].index == 1); + EXPECT_TRUE(mutations3[1].type == ShadowViewMutation::Delete); + EXPECT_TRUE(mutations3[1].oldChildShadowView.tag == 102); + EXPECT_TRUE(mutations3[2].type == ShadowViewMutation::Create); + EXPECT_TRUE(mutations3[2].newChildShadowView.tag == 104); + EXPECT_TRUE(mutations3[3].type == ShadowViewMutation::Insert); + EXPECT_TRUE(mutations3[3].newChildShadowView.tag == 104); + EXPECT_TRUE(mutations3[3].index == 2); // Calculating mutations. - auto mutations4 = calculateShadowViewMutations(*rootNodeV4, *rootNodeV5); + auto mutations4 = + calculateShadowViewMutations(*rootNodeV4, *rootNodeV5, false); // The order and exact mutation instructions here may change at any time. // This test just ensures that any changes are intentional. @@ -288,43 +306,45 @@ TEST(MountingTest, testMinimalInstructionGeneration) { // at the end, and removing a node in the middle, produces the minimal set of // instructions. All these nodes are laid out with absolute positioning, so // moving them around does not change layout. - assert(mutations4.size() == 6); - assert(mutations4[0].type == ShadowViewMutation::Remove); - assert(mutations4[0].oldChildShadowView.tag == 103); - assert(mutations4[0].index == 1); - assert(mutations4[1].type == ShadowViewMutation::Delete); - assert(mutations4[1].oldChildShadowView.tag == 103); - assert(mutations4[2].type == ShadowViewMutation::Create); - assert(mutations4[2].newChildShadowView.tag == 100); - assert(mutations4[3].type == ShadowViewMutation::Create); - assert(mutations4[3].newChildShadowView.tag == 102); - assert(mutations4[4].type == ShadowViewMutation::Insert); - assert(mutations4[4].newChildShadowView.tag == 100); - assert(mutations4[4].index == 1); - assert(mutations4[5].type == ShadowViewMutation::Insert); - assert(mutations4[5].newChildShadowView.tag == 102); - assert(mutations4[5].index == 3); - - auto mutations5 = calculateShadowViewMutations(*rootNodeV5, *rootNodeV6); + EXPECT_TRUE(mutations4.size() == 6); + EXPECT_TRUE(mutations4[0].type == ShadowViewMutation::Remove); + EXPECT_TRUE(mutations4[0].oldChildShadowView.tag == 103); + EXPECT_TRUE(mutations4[0].index == 1); + EXPECT_TRUE(mutations4[1].type == ShadowViewMutation::Delete); + EXPECT_TRUE(mutations4[1].oldChildShadowView.tag == 103); + EXPECT_TRUE(mutations4[2].type == ShadowViewMutation::Create); + EXPECT_TRUE(mutations4[2].newChildShadowView.tag == 100); + EXPECT_TRUE(mutations4[3].type == ShadowViewMutation::Create); + EXPECT_TRUE(mutations4[3].newChildShadowView.tag == 102); + EXPECT_TRUE(mutations4[4].type == ShadowViewMutation::Insert); + EXPECT_TRUE(mutations4[4].newChildShadowView.tag == 100); + EXPECT_TRUE(mutations4[4].index == 1); + EXPECT_TRUE(mutations4[5].type == ShadowViewMutation::Insert); + EXPECT_TRUE(mutations4[5].newChildShadowView.tag == 102); + EXPECT_TRUE(mutations4[5].index == 3); + + auto mutations5 = + calculateShadowViewMutations(*rootNodeV5, *rootNodeV6, false); // The order and exact mutation instructions here may change at any time. // This test just ensures that any changes are intentional. // This test, in particular, ensures that inserting TWO children in the middle // produces the minimal set of instructions. All these nodes are laid out with // absolute positioning, so moving them around does not change layout. - assert(mutations5.size() == 4); - assert(mutations5[0].type == ShadowViewMutation::Create); - assert(mutations5[0].newChildShadowView.tag == 103); - assert(mutations5[1].type == ShadowViewMutation::Create); - assert(mutations5[1].newChildShadowView.tag == 105); - assert(mutations5[2].type == ShadowViewMutation::Insert); - assert(mutations5[2].newChildShadowView.tag == 103); - assert(mutations5[2].index == 2); - assert(mutations5[3].type == ShadowViewMutation::Insert); - assert(mutations5[3].newChildShadowView.tag == 105); - assert(mutations5[3].index == 3); - - auto mutations6 = calculateShadowViewMutations(*rootNodeV6, *rootNodeV7); + EXPECT_TRUE(mutations5.size() == 4); + EXPECT_TRUE(mutations5[0].type == ShadowViewMutation::Create); + EXPECT_TRUE(mutations5[0].newChildShadowView.tag == 103); + EXPECT_TRUE(mutations5[1].type == ShadowViewMutation::Create); + EXPECT_TRUE(mutations5[1].newChildShadowView.tag == 105); + EXPECT_TRUE(mutations5[2].type == ShadowViewMutation::Insert); + EXPECT_TRUE(mutations5[2].newChildShadowView.tag == 103); + EXPECT_TRUE(mutations5[2].index == 2); + EXPECT_TRUE(mutations5[3].type == ShadowViewMutation::Insert); + EXPECT_TRUE(mutations5[3].newChildShadowView.tag == 105); + EXPECT_TRUE(mutations5[3].index == 3); + + auto mutations6 = + calculateShadowViewMutations(*rootNodeV6, *rootNodeV7, false); // The order and exact mutation instructions here may change at any time. // This test just ensures that any changes are intentional. @@ -333,13 +353,333 @@ TEST(MountingTest, testMinimalInstructionGeneration) { // create more "CREATE" mutations than necessary. // The actual nodes that should be created in this transaction have a tag > // 105. - assert(mutations6.size() == 25); + EXPECT_TRUE(mutations6.size() == 25); for (int i = 0; i < mutations6.size(); i++) { if (mutations6[i].type == ShadowViewMutation::Create) { - assert(mutations6[i].newChildShadowView.tag > 105); + EXPECT_TRUE(mutations6[i].newChildShadowView.tag > 105); } } } +/** + * Test reparenting mutation instruction generation. + */ +TEST(MountingTest, testViewReparentingInstructionGeneration) { + auto eventDispatcher = EventDispatcher::Shared{}; + auto contextContainer = std::make_shared(); + auto componentDescriptorParameters = + ComponentDescriptorParameters{eventDispatcher, contextContainer, nullptr}; + auto viewComponentDescriptor = + ViewComponentDescriptor(componentDescriptorParameters); + auto rootComponentDescriptor = + RootComponentDescriptor(componentDescriptorParameters); + + auto rootFamily = rootComponentDescriptor.createFamily( + {Tag(1), SurfaceId(1), nullptr}, nullptr); + + // Creating an initial root shadow node. + auto emptyRootNode = std::const_pointer_cast( + std::static_pointer_cast( + rootComponentDescriptor.createShadowNode( + ShadowNodeFragment{RootShadowNode::defaultSharedProps()}, + rootFamily))); + + // Applying size constraints. + emptyRootNode = emptyRootNode->clone( + LayoutConstraints{Size{512, 0}, + Size{512, std::numeric_limits::infinity()}}, + LayoutContext{}); + + auto childA = makeNode(viewComponentDescriptor, 100, {}); + auto childB = makeNode(viewComponentDescriptor, 101, {}); + auto childC = makeNode(viewComponentDescriptor, 102, {}); + auto childD = makeNode(viewComponentDescriptor, 103, {}); + auto childE = makeNode(viewComponentDescriptor, 104, {}); + auto childF = makeNode(viewComponentDescriptor, 105, {}); + + auto childG = makeNode(viewComponentDescriptor, 106, {}); + auto childH = makeNode(viewComponentDescriptor, 107, {}); + auto childI = makeNode(viewComponentDescriptor, 108, {}); + auto childJ = makeNode(viewComponentDescriptor, 109, {}); + auto childK = makeNode(viewComponentDescriptor, 110, {}); + + auto family = viewComponentDescriptor.createFamily( + {10, SurfaceId(1), nullptr}, nullptr); + + auto reparentedViewA = makeNode( + viewComponentDescriptor, + 1000, + SharedShadowNodeList{ + childC->clone({}), childA->clone({}), childB->clone({})}); + auto reparentedViewB = makeNode( + viewComponentDescriptor, + 2000, + SharedShadowNodeList{ + childF->clone({}), childE->clone({}), childD->clone({})}); + + // Root -> G* -> H -> I -> J -> A* [nodes with * are _not_ flattened] + auto shadowNodeV1 = viewComponentDescriptor.createShadowNode(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childG->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childH->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childI->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared< + SharedShadowNodeList>(SharedShadowNodeList{ + childJ->clone(ShadowNodeFragment{ + generateDefaultProps( + viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{ + reparentedViewA->clone({})})})})})})})})})})}, family); + + // Root -> G* -> H* -> I -> J -> A* [nodes with * are _not_ flattened] + auto shadowNodeV2 = shadowNodeV1->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childG->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childH->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childI->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared< + SharedShadowNodeList>(SharedShadowNodeList{ + childJ->clone(ShadowNodeFragment{ + generateDefaultProps( + viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{ + reparentedViewA->clone({})})})})})})})})})})}); + + // Root -> G* -> H -> I -> J -> A* [nodes with * are _not_ flattened] + auto shadowNodeV3 = shadowNodeV2->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childG->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childH->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childI->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared< + SharedShadowNodeList>(SharedShadowNodeList{ + childJ->clone(ShadowNodeFragment{ + generateDefaultProps( + viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{ + reparentedViewA->clone({})})})})})})})})})})}); + + // The view is reparented 1 level down with a different sibling + // Root -> G* -> H* -> I* -> J -> [B*, A*] [nodes with * are _not_ flattened] + auto shadowNodeV4 = shadowNodeV3->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childG->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childH->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childI->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared< + SharedShadowNodeList>(SharedShadowNodeList{ + childJ->clone(ShadowNodeFragment{ + generateDefaultProps( + viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{ + reparentedViewB->clone({}), + reparentedViewA->clone({})})})})})})})})})})}); + + // The view is reparented 1 level further down with its order with the sibling + // swapped + // Root -> G* -> H* -> I* -> J* -> [A*, B*] [nodes with * are _not_ flattened] + auto shadowNodeV5 = shadowNodeV4->clone(ShadowNodeFragment{ + generateDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childG->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childH->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{childI->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps(viewComponentDescriptor), + std::make_shared< + SharedShadowNodeList>(SharedShadowNodeList{ + childJ->clone(ShadowNodeFragment{ + nonFlattenedDefaultProps( + viewComponentDescriptor), + std::make_shared( + SharedShadowNodeList{ + reparentedViewA->clone({}), + reparentedViewB->clone({}) + })})})})})})})})})}); + + // Injecting a tree into the root node. + auto rootNodeV1 = std::static_pointer_cast( + emptyRootNode->ShadowNode::clone( + ShadowNodeFragment{ShadowNodeFragment::propsPlaceholder(), + std::make_shared( + SharedShadowNodeList{shadowNodeV1})})); + auto rootNodeV2 = std::static_pointer_cast( + rootNodeV1->ShadowNode::clone( + ShadowNodeFragment{ShadowNodeFragment::propsPlaceholder(), + std::make_shared( + SharedShadowNodeList{shadowNodeV2})})); + auto rootNodeV3 = std::static_pointer_cast( + rootNodeV2->ShadowNode::clone( + ShadowNodeFragment{ShadowNodeFragment::propsPlaceholder(), + std::make_shared( + SharedShadowNodeList{shadowNodeV3})})); + auto rootNodeV4 = std::static_pointer_cast( + rootNodeV3->ShadowNode::clone( + ShadowNodeFragment{ShadowNodeFragment::propsPlaceholder(), + std::make_shared( + SharedShadowNodeList{shadowNodeV4})})); + auto rootNodeV5 = std::static_pointer_cast( + rootNodeV4->ShadowNode::clone( + ShadowNodeFragment{ShadowNodeFragment::propsPlaceholder(), + std::make_shared( + SharedShadowNodeList{shadowNodeV5})})); + + // Layout + std::vector affectedLayoutableNodesV1{}; + affectedLayoutableNodesV1.reserve(1024); + std::const_pointer_cast(rootNodeV1) + ->layoutIfNeeded(&affectedLayoutableNodesV1); + rootNodeV1->sealRecursive(); + + std::vector affectedLayoutableNodesV2{}; + affectedLayoutableNodesV2.reserve(1024); + std::const_pointer_cast(rootNodeV2) + ->layoutIfNeeded(&affectedLayoutableNodesV2); + rootNodeV2->sealRecursive(); + + std::vector affectedLayoutableNodesV3{}; + affectedLayoutableNodesV3.reserve(1024); + std::const_pointer_cast(rootNodeV3) + ->layoutIfNeeded(&affectedLayoutableNodesV3); + rootNodeV3->sealRecursive(); + + std::vector affectedLayoutableNodesV4{}; + affectedLayoutableNodesV4.reserve(1024); + std::const_pointer_cast(rootNodeV4) + ->layoutIfNeeded(&affectedLayoutableNodesV4); + rootNodeV4->sealRecursive(); + + std::vector affectedLayoutableNodesV5{}; + affectedLayoutableNodesV5.reserve(1024); + std::const_pointer_cast(rootNodeV5) + ->layoutIfNeeded(&affectedLayoutableNodesV5); + rootNodeV5->sealRecursive(); + + // Calculating mutations. + auto mutations1 = + calculateShadowViewMutations(*rootNodeV1, *rootNodeV2, true); + + EXPECT_EQ(mutations1.size(), 5); + EXPECT_EQ(mutations1[0].type, ShadowViewMutation::Update); + EXPECT_EQ(mutations1[0].oldChildShadowView.tag, 106); + EXPECT_EQ(mutations1[1].type, ShadowViewMutation::Remove); + EXPECT_EQ(mutations1[1].oldChildShadowView.tag, 1000); + EXPECT_EQ(mutations1[2].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations1[2].newChildShadowView.tag, 107); + EXPECT_EQ(mutations1[3].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations1[3].newChildShadowView.tag, 1000); + EXPECT_EQ(mutations1[4].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations1[4].newChildShadowView.tag, 107); + + auto mutations2 = + calculateShadowViewMutations(*rootNodeV2, *rootNodeV3, true); + + EXPECT_EQ(mutations2.size(), 5); + EXPECT_EQ(mutations2[0].type, ShadowViewMutation::Update); + EXPECT_EQ(mutations2[0].oldChildShadowView.tag, 106); + EXPECT_EQ(mutations2[1].type, ShadowViewMutation::Remove); + EXPECT_EQ(mutations2[1].oldChildShadowView.tag, 1000); + EXPECT_EQ(mutations2[2].type, ShadowViewMutation::Remove); + EXPECT_EQ(mutations2[2].oldChildShadowView.tag, 107); + EXPECT_EQ( + mutations2[3].type, + ShadowViewMutation::Delete); // correct, 107 is removed from tree entirely + EXPECT_EQ(mutations2[3].oldChildShadowView.tag, 107); + EXPECT_EQ(mutations2[4].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations2[4].newChildShadowView.tag, 1000); + + auto mutations3 = + calculateShadowViewMutations(*rootNodeV3, *rootNodeV4, true); + + // between these two trees, lots of new nodes are created and inserted - this + // is all correct, and this is the minimal amount of mutations + + EXPECT_EQ(mutations3.size(), 15); + EXPECT_EQ(mutations3[0].type, ShadowViewMutation::Update); + EXPECT_EQ(mutations3[0].oldChildShadowView.tag, 106); + EXPECT_EQ(mutations3[1].type, ShadowViewMutation::Remove); + EXPECT_EQ(mutations3[1].oldChildShadowView.tag, 1000); + EXPECT_EQ(mutations3[2].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations3[2].newChildShadowView.tag, 107); + EXPECT_EQ(mutations3[3].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations3[3].newChildShadowView.tag, 108); + EXPECT_EQ(mutations3[4].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations3[4].newChildShadowView.tag, 2000); + EXPECT_EQ(mutations3[5].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations3[5].newChildShadowView.tag, 105); + EXPECT_EQ(mutations3[6].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations3[6].newChildShadowView.tag, 104); + EXPECT_EQ(mutations3[7].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations3[7].newChildShadowView.tag, 103); + EXPECT_EQ(mutations3[8].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[8].newChildShadowView.tag, 105); + EXPECT_EQ(mutations3[9].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[9].newChildShadowView.tag, 104); + EXPECT_EQ(mutations3[10].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[10].newChildShadowView.tag, 103); + EXPECT_EQ(mutations3[11].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[11].newChildShadowView.tag, 2000); + EXPECT_EQ(mutations3[12].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[12].newChildShadowView.tag, 1000); + EXPECT_EQ(mutations3[13].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[13].newChildShadowView.tag, 108); + EXPECT_EQ(mutations3[14].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations3[14].newChildShadowView.tag, 107); + + auto mutations4 = + calculateShadowViewMutations(*rootNodeV4, *rootNodeV5, true); + + EXPECT_EQ(mutations4.size(), 9); + EXPECT_EQ(mutations4[0].type, ShadowViewMutation::Update); + EXPECT_EQ(mutations4[0].oldChildShadowView.tag, 106); + EXPECT_EQ(mutations4[1].type, ShadowViewMutation::Update); + EXPECT_EQ(mutations4[1].oldChildShadowView.tag, 107); + EXPECT_EQ(mutations4[2].type, ShadowViewMutation::Update); + EXPECT_EQ(mutations4[2].oldChildShadowView.tag, 108); + EXPECT_EQ(mutations4[3].type, ShadowViewMutation::Remove); + EXPECT_EQ(mutations4[3].oldChildShadowView.tag, 1000); + EXPECT_EQ(mutations4[4].type, ShadowViewMutation::Remove); + EXPECT_EQ(mutations4[4].oldChildShadowView.tag, 2000); + EXPECT_EQ(mutations4[5].type, ShadowViewMutation::Create); + EXPECT_EQ(mutations4[5].newChildShadowView.tag, 109); + EXPECT_EQ(mutations4[6].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations4[6].newChildShadowView.tag, 1000); + EXPECT_EQ(mutations4[7].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations4[7].newChildShadowView.tag, 2000); + EXPECT_EQ(mutations4[8].type, ShadowViewMutation::Insert); + EXPECT_EQ(mutations4[8].newChildShadowView.tag, 109); +} + } // namespace react } // namespace facebook diff --git a/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 6b251264267cdf..58128870bd0a7c 100644 --- a/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -106,11 +106,15 @@ Scheduler::Scheduler( uiManager_->setAnimationDelegate(animationDelegate); #ifdef ANDROID + enableReparentingDetection_ = reactNativeConfig_->getBool( + "react_fabric:enable_reparenting_detection_android"); enableNewStateReconciliation_ = reactNativeConfig_->getBool( "react_fabric:enable_new_state_reconciliation_android"); removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool( "react_fabric:remove_outstanding_surfaces_on_destruction_android"); #else + enableReparentingDetection_ = reactNativeConfig_->getBool( + "react_fabric:enable_reparenting_detection_ios"); enableNewStateReconciliation_ = reactNativeConfig_->getBool( "react_fabric:enable_new_state_reconciliation_ios"); removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool( @@ -185,7 +189,8 @@ void Scheduler::startSurface( layoutContext, *rootComponentDescriptor_, *uiManager_, - mountingOverrideDelegate); + mountingOverrideDelegate, + enableReparentingDetection_); shadowTree->setEnableNewStateReconciliation(enableNewStateReconciliation_); diff --git a/ReactCommon/react/renderer/scheduler/Scheduler.h b/ReactCommon/react/renderer/scheduler/Scheduler.h index 86496494d6d30d..c025b39a3b98dd 100644 --- a/ReactCommon/react/renderer/scheduler/Scheduler.h +++ b/ReactCommon/react/renderer/scheduler/Scheduler.h @@ -137,6 +137,7 @@ class Scheduler final : public UIManagerDelegate { /* * Temporary flags. */ + bool enableReparentingDetection_{false}; bool enableNewStateReconciliation_{false}; bool removeOutstandingSurfacesOnDestruction_{false}; };