From 756867933e65f48fde06921c6ead66122f3152a7 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Fri, 27 Sep 2024 13:11:45 -0700 Subject: [PATCH] Fabric: Fixes animations strict weak ordering sorted check failed (#46582) Summary: Fixes https://github.com/facebook/react-native/issues/46568 . cc cipolleschi ## Changelog: [IOS] [FIXED] - Fabric: Fixes animations strict weak ordering sorted check failed Pull Request resolved: https://github.com/facebook/react-native/pull/46582 Test Plan: See issue in https://github.com/facebook/react-native/issues/46568 ## Repro steps - Install Xcode 16.0 - navigate to react-native-github - yarn install - cd packages/rn-tester - bundle install - RCT_NEW_ARCH_ENABLED=1 bundle exec pod install open RNTesterPods.xcworkspace to open Xcode {F1885373361} Testing with Reproducer from OSS | Paper | Fabric (With Fix) | |--------|-----------------| | {F1885395747} | {F1885395870} | Android - LayoutAnimation (Looks like it has been broken and not working way before this changes.) https://pxl.cl/5DGVv Reviewed By: cipolleschi Differential Revision: D63399017 Pulled By: realsoelynn fbshipit-source-id: aaf4ac2884ccca2da7e90a52a8ef10df6ae4fc8a --- .../LayoutAnimationKeyFrameManager.cpp | 5 +-- .../react/renderer/animations/utils.h | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp b/packages/react-native/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp index bcf665fcdac367..e90f7246d52cd4 100644 --- a/packages/react-native/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp @@ -790,10 +790,7 @@ LayoutAnimationKeyFrameManager::pullTransaction( finalConflictingMutations.end(), &shouldFirstComeBeforeSecondMutation); - std::stable_sort( - immediateMutations.begin(), - immediateMutations.end(), - &shouldFirstComeBeforeSecondRemovesOnly); + handleShouldFirstComeBeforeSecondRemovesOnly(immediateMutations); animation.keyFrames = keyFramesToAnimate; inflightAnimations_.push_back(std::move(animation)); diff --git a/packages/react-native/ReactCommon/react/renderer/animations/utils.h b/packages/react-native/ReactCommon/react/renderer/animations/utils.h index 5ddd226a71eee6..41b9be93b0a230 100644 --- a/packages/react-native/ReactCommon/react/renderer/animations/utils.h +++ b/packages/react-native/ReactCommon/react/renderer/animations/utils.h @@ -24,6 +24,40 @@ static inline bool shouldFirstComeBeforeSecondRemovesOnly( (lhs.index > rhs.index); } +static inline void handleShouldFirstComeBeforeSecondRemovesOnly( + ShadowViewMutation::List& list) noexcept { + std::unordered_map> + removeMutationsByTag; + ShadowViewMutation::List finalList; + for (auto& mutation : list) { + if (mutation.type == ShadowViewMutation::Type::Remove) { + auto key = std::to_string(mutation.parentShadowView.tag); + removeMutationsByTag[key].push_back(mutation); + } else { + finalList.push_back(mutation); + } + } + + if (removeMutationsByTag.size() == 0) { + return; + } + + for (auto& mutationsPair : removeMutationsByTag) { + if (mutationsPair.second.size() > 1) { + std::stable_sort( + mutationsPair.second.begin(), + mutationsPair.second.end(), + &shouldFirstComeBeforeSecondRemovesOnly); + } + finalList.insert( + finalList.begin(), + mutationsPair.second.begin(), + mutationsPair.second.end()); + } + + list = finalList; +} + static inline bool shouldFirstComeBeforeSecondMutation( const ShadowViewMutation& lhs, const ShadowViewMutation& rhs) noexcept { @@ -55,6 +89,17 @@ static inline bool shouldFirstComeBeforeSecondMutation( lhs.type == ShadowViewMutation::Type::Insert) { return false; } + + // Remove comes before Update + if (lhs.type == ShadowViewMutation::Type::Remove && + rhs.type == ShadowViewMutation::Type::Update) { + return true; + } + if (rhs.type == ShadowViewMutation::Type::Remove && + lhs.type == ShadowViewMutation::Type::Update) { + return false; + } + } else { // Make sure that removes on the same level are sorted - highest indices // must come first.