From 07a7b98f6475c94536d03adf1987256c35269397 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 22 Nov 2021 13:48:33 -0500 Subject: [PATCH 01/13] Fixes issue with restarting animations Animated Composition values cannot be queried until the animation completes (generally, 1 full frame callback after the animation is stopped). The value is also ready to be queried after a scoped batch completion event fires. This change introduces deferred animation starts. If we attempt to start an animation on a ValueAnimatedNode where an active animation is already running, we wait for the `Completed` callback on that animations scoped batch to fire before starting the next animation. The `Completed` callback will fire either after the animation has run to completion or roughly one animation frame after `StopAnimation` is called. Doing this ensures that we call `FlattenOffset` so the starting value for the ValueAnimatedNode is correct when we start the animation next. Fixes #8419 --- .../Modules/Animated/AnimationDriver.cpp | 27 +++++++++++-------- .../Modules/Animated/AnimationDriver.h | 2 ++ .../Animated/NativeAnimatedNodeManager.cpp | 8 +++++- .../Animated/NativeAnimatedNodeManager.h | 2 ++ .../Modules/Animated/ValueAnimatedNode.cpp | 17 ++++++++++++ .../Modules/Animated/ValueAnimatedNode.h | 4 +++ 6 files changed, 48 insertions(+), 12 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index 9caf25db30d..dbecce07870 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -42,8 +42,14 @@ AnimationDriver::~AnimationDriver() { } void AnimationDriver::StartAnimation() { + auto const animatedValue = GetAnimatedValue(); + if (animatedValue && animatedValue->HasActiveAnimations()) { + animatedValue->DeferAnimation(m_id); + return; + } + const auto [animation, scopedBatch] = MakeAnimation(m_config); - if (auto const animatedValue = GetAnimatedValue()) { + if (animatedValue) { animatedValue->PropertySet().StartAnimation(ValueAnimatedNode::s_valueName, animation); animatedValue->AddActiveAnimation(m_id); } @@ -52,7 +58,13 @@ void AnimationDriver::StartAnimation() { m_scopedBatchCompletedToken = scopedBatch.Completed( [weakSelf = weak_from_this(), weakManager = m_manager, id = m_id, tag = m_animatedValueTag](auto sender, auto) { if (const auto strongSelf = weakSelf.lock()) { - strongSelf->DoCallback(true); + if (strongSelf->m_ignoreCompletedHandlers) { + return; + } + + strongSelf->DoCallback(strongSelf->m_stopped); + strongSelf->m_scopedBatch.Completed(strongSelf->m_scopedBatchCompletedToken); + strongSelf->m_scopedBatch = nullptr; } if (auto manager = weakManager.lock()) { @@ -70,15 +82,8 @@ void AnimationDriver::StartAnimation() { void AnimationDriver::StopAnimation(bool ignoreCompletedHandlers) { if (const auto animatedValue = GetAnimatedValue()) { animatedValue->PropertySet().StopAnimation(ValueAnimatedNode::s_valueName); - if (!ignoreCompletedHandlers) { - animatedValue->RemoveActiveAnimation(m_id); - - if (m_scopedBatch) { - DoCallback(false); - m_scopedBatch.Completed(m_scopedBatchCompletedToken); - m_scopedBatch = nullptr; - } - } + m_stopped = true; + m_ignoreCompletedHandlers = ignoreCompletedHandlers; } } diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h index d24c7422c49..937d4eafeb4 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h @@ -73,5 +73,7 @@ class AnimationDriver : public std::enable_shared_from_this { // auto revoker for scopedBatch.Completed is broken, tracked by internal bug // #22399779 winrt::event_token m_scopedBatchCompletedToken{}; + bool m_stopped{false}; + bool m_ignoreCompletedHandlers{false}; }; } // namespace Microsoft::ReactNative diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 25c9451b44e..34bf0dd6340 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -124,7 +124,6 @@ void NativeAnimatedNodeManager::StopAnimation(int64_t animationId) { if (m_activeAnimations.count(animationId)) { if (const auto animation = m_activeAnimations.at(animationId).get()) { animation->StopAnimation(); - m_activeAnimations.erase(animationId); } } } @@ -403,6 +402,13 @@ TrackingAnimatedNode *NativeAnimatedNodeManager::GetTrackingAnimatedNode(int64_t return nullptr; } +AnimationDriver* NativeAnimatedNodeManager::GetActiveAnimation(int64_t tag) { + if (m_activeAnimations.count(tag)) { + return m_activeAnimations.at(tag).get(); + } + return nullptr; +} + void NativeAnimatedNodeManager::RemoveActiveAnimation(int64_t tag) { m_activeAnimations.erase(tag); } diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h index 8cae2a1d426..3344e510cc2 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h @@ -88,6 +88,8 @@ class NativeAnimatedNodeManager { StyleAnimatedNode *GetStyleAnimatedNode(int64_t tag); TransformAnimatedNode *GetTransformAnimatedNode(int64_t tag); TrackingAnimatedNode *GetTrackingAnimatedNode(int64_t tag); + + AnimationDriver *GetActiveAnimation(int64_t tag); void RemoveActiveAnimation(int64_t tag); private: diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp index f27f42557b7..7ea8d28855b 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp @@ -98,10 +98,19 @@ void ValueAnimatedNode::RemoveActiveAnimation(int64_t animationTag) { m_activeAnimations.erase(animationTag); if (!m_activeAnimations.size()) { if (const auto manager = m_manager.lock()) { + // Dispose completed props animations for (const auto &props : m_dependentPropsNodes) { if (const auto propsNode = manager->GetPropsAnimatedNode(props)) propsNode->DisposeCompletedAnimation(Tag()); } + + // Start any deferred animations + for (const auto &deferredId : m_deferredAnimations) { + if (const auto animationDriver = manager->GetActiveAnimation(deferredId)) { + animationDriver->StartAnimation(); + } + } + m_deferredAnimations.clear(); } } } @@ -114,6 +123,14 @@ void ValueAnimatedNode::RemoveActiveTrackingNode(int64_t trackingNodeTag) { m_activeTrackingNodes.erase(trackingNodeTag); } +bool ValueAnimatedNode::HasActiveAnimations() const noexcept { + return m_activeAnimations.size(); +} + +void ValueAnimatedNode::DeferAnimation(int64_t animationTag) { + m_deferredAnimations.insert(animationTag); +} + void ValueAnimatedNode::UpdateTrackingNodes() { if (auto const manager = m_manager.lock()) { for (auto trackingNodeTag : m_activeTrackingNodes) { diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h index b039e6276d7..ec78cff8605 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h @@ -36,6 +36,9 @@ class ValueAnimatedNode : public AnimatedNode { void AddActiveTrackingNode(int64_t trackingNodeTag); void RemoveActiveTrackingNode(int64_t trackingNodeTag); + void DeferAnimation(int64_t animationTag); + bool HasActiveAnimations() const noexcept; + static constexpr std::wstring_view s_valueName{L"v"}; static constexpr std::wstring_view s_offsetName{L"o"}; @@ -52,5 +55,6 @@ class ValueAnimatedNode : public AnimatedNode { std::unordered_set m_dependentPropsNodes{}; std::unordered_set m_activeAnimations{}; std::unordered_set m_activeTrackingNodes{}; + std::unordered_set m_deferredAnimations{}; }; } // namespace Microsoft::ReactNative From e65b793797fafd1a10adf6cd222745501ea09805 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 22 Nov 2021 13:56:24 -0500 Subject: [PATCH 02/13] Change files --- ...ative-windows-1ea926cf-a734-42b3-84e6-d1254a7676f1.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/react-native-windows-1ea926cf-a734-42b3-84e6-d1254a7676f1.json diff --git a/change/react-native-windows-1ea926cf-a734-42b3-84e6-d1254a7676f1.json b/change/react-native-windows-1ea926cf-a734-42b3-84e6-d1254a7676f1.json new file mode 100644 index 00000000000..7c8e3196445 --- /dev/null +++ b/change/react-native-windows-1ea926cf-a734-42b3-84e6-d1254a7676f1.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Fixes issue with restarting animations", + "packageName": "react-native-windows", + "email": "erozell@outlook.com", + "dependentChangeType": "patch" +} From 1dcf0fa9dcc6c22a4ac3766b72c0e14362782a96 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 22 Nov 2021 14:20:37 -0500 Subject: [PATCH 03/13] Puts ValueAnimatedNode in control of the animation driver it is deferring animations for. --- .../Modules/Animated/AnimationDriver.cpp | 6 +++++- .../Modules/Animated/NativeAnimatedNodeManager.cpp | 1 + .../Modules/Animated/ValueAnimatedNode.cpp | 7 ++++--- .../Modules/Animated/ValueAnimatedNode.h | 5 +++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index dbecce07870..2d49065f1bc 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -51,7 +51,10 @@ void AnimationDriver::StartAnimation() { const auto [animation, scopedBatch] = MakeAnimation(m_config); if (animatedValue) { animatedValue->PropertySet().StartAnimation(ValueAnimatedNode::s_valueName, animation); - animatedValue->AddActiveAnimation(m_id); + + // The ValueAnimatedNode needs to retain a reference to this animation driver + // so it can be kept alive until the scoped batch completion callback fires. + animatedValue->AddActiveAnimation(shared_from_this()); } scopedBatch.End(); @@ -81,6 +84,7 @@ void AnimationDriver::StartAnimation() { void AnimationDriver::StopAnimation(bool ignoreCompletedHandlers) { if (const auto animatedValue = GetAnimatedValue()) { + animatedValue->StopAnimation(m_id); animatedValue->PropertySet().StopAnimation(ValueAnimatedNode::s_valueName); m_stopped = true; m_ignoreCompletedHandlers = ignoreCompletedHandlers; diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 34bf0dd6340..8d1b2a042b1 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -124,6 +124,7 @@ void NativeAnimatedNodeManager::StopAnimation(int64_t animationId) { if (m_activeAnimations.count(animationId)) { if (const auto animation = m_activeAnimations.at(animationId).get()) { animation->StopAnimation(); + m_activeAnimations.erase(animationId); } } } diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp index 7ea8d28855b..7b1c03bc0cf 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp @@ -82,8 +82,8 @@ void ValueAnimatedNode::RemoveDependentPropsNode(int64_t propsNodeTag) { m_dependentPropsNodes.erase(propsNodeTag); } -void ValueAnimatedNode::AddActiveAnimation(int64_t animationTag) { - m_activeAnimations.insert(animationTag); +void ValueAnimatedNode::AddActiveAnimation(std::shared_ptr animation) { + m_activeAnimations.insert({animation->Id(), animation}); if (m_activeAnimations.size() == 1) { if (const auto manager = m_manager.lock()) { for (const auto &props : m_dependentPropsNodes) { @@ -104,7 +104,8 @@ void ValueAnimatedNode::RemoveActiveAnimation(int64_t animationTag) { propsNode->DisposeCompletedAnimation(Tag()); } - // Start any deferred animations + // Start any deferred animations, if the animation was stopped before + // this is run, we will get a null pointer for the animation driver. for (const auto &deferredId : m_deferredAnimations) { if (const auto animationDriver = manager->GetActiveAnimation(deferredId)) { animationDriver->StartAnimation(); diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h index ec78cff8605..0f54fdb2a79 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h @@ -11,6 +11,7 @@ using namespace comp; } namespace Microsoft::ReactNative { +class AnimationDriver; class ValueAnimatedNode : public AnimatedNode { public: ValueAnimatedNode( @@ -31,7 +32,7 @@ class ValueAnimatedNode : public AnimatedNode { void AddDependentPropsNode(int64_t propsNodeTag); void RemoveDependentPropsNode(int64_t propsNodeTag); - void AddActiveAnimation(int64_t animationTag); + void AddActiveAnimation(std::shared_ptr animation); void RemoveActiveAnimation(int64_t animationTag); void AddActiveTrackingNode(int64_t trackingNodeTag); void RemoveActiveTrackingNode(int64_t trackingNodeTag); @@ -52,8 +53,8 @@ class ValueAnimatedNode : public AnimatedNode { private: void UpdateTrackingNodes(); + std::unordered_map> m_activeAnimations{}; std::unordered_set m_dependentPropsNodes{}; - std::unordered_set m_activeAnimations{}; std::unordered_set m_activeTrackingNodes{}; std::unordered_set m_deferredAnimations{}; }; From 461ff18aaca93d98d9b8f7843245f1619a447afe Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 22 Nov 2021 14:38:23 -0500 Subject: [PATCH 04/13] Only defer animations if a previous animation was stopped. --- .../Modules/Animated/AnimationDriver.cpp | 6 +++--- .../Modules/Animated/ValueAnimatedNode.cpp | 20 ++++++++++++++++--- .../Modules/Animated/ValueAnimatedNode.h | 4 +++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index 2d49065f1bc..f2604dd1d19 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -43,7 +43,9 @@ AnimationDriver::~AnimationDriver() { void AnimationDriver::StartAnimation() { auto const animatedValue = GetAnimatedValue(); - if (animatedValue && animatedValue->HasActiveAnimations()) { + // If the animated value has any stopped animations, we must wait for a completion + // callback on these animations to ensure the latest value is correct. + if (animatedValue && animatedValue->HasStoppedAnimations()) { animatedValue->DeferAnimation(m_id); return; } @@ -66,8 +68,6 @@ void AnimationDriver::StartAnimation() { } strongSelf->DoCallback(strongSelf->m_stopped); - strongSelf->m_scopedBatch.Completed(strongSelf->m_scopedBatchCompletedToken); - strongSelf->m_scopedBatch = nullptr; } if (auto manager = weakManager.lock()) { diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp index 7b1c03bc0cf..4a7a59a0a69 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp @@ -96,6 +96,7 @@ void ValueAnimatedNode::AddActiveAnimation(std::shared_ptr anim void ValueAnimatedNode::RemoveActiveAnimation(int64_t animationTag) { m_activeAnimations.erase(animationTag); + m_stoppedAnimations.erase(animationTag); if (!m_activeAnimations.size()) { if (const auto manager = m_manager.lock()) { // Dispose completed props animations @@ -105,7 +106,8 @@ void ValueAnimatedNode::RemoveActiveAnimation(int64_t animationTag) { } // Start any deferred animations, if the animation was stopped before - // this is run, we will get a null pointer for the animation driver. + // this is run, we will get a null pointer for the animation driver. We + // also remove any deferred animation tags in StopAnimation. for (const auto &deferredId : m_deferredAnimations) { if (const auto animationDriver = manager->GetActiveAnimation(deferredId)) { animationDriver->StartAnimation(); @@ -124,8 +126,8 @@ void ValueAnimatedNode::RemoveActiveTrackingNode(int64_t trackingNodeTag) { m_activeTrackingNodes.erase(trackingNodeTag); } -bool ValueAnimatedNode::HasActiveAnimations() const noexcept { - return m_activeAnimations.size(); +bool ValueAnimatedNode::HasStoppedAnimations() const noexcept { + return m_stoppedAnimations.size(); } void ValueAnimatedNode::DeferAnimation(int64_t animationTag) { @@ -142,4 +144,16 @@ void ValueAnimatedNode::UpdateTrackingNodes() { } } +void ValueAnimatedNode::StopAnimation(int64_t animationTag) { + if (m_deferredAnimations.count(animationTag)) { + // This is not strictly necessary as the animation will be destroyed by the + // NativeAnimatedNodesManager if it was stopped before its deferred start. + m_deferredAnimations.erase(animationTag); + } else { + // The animation should already be running if it is not currently deferred. + assert(m_activeAnimations.count(animationTag)); + m_stoppedAnimations.insert(animationTag); + } +} + } // namespace Microsoft::ReactNative diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h index 0f54fdb2a79..7428a14abdc 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h @@ -38,7 +38,8 @@ class ValueAnimatedNode : public AnimatedNode { void RemoveActiveTrackingNode(int64_t trackingNodeTag); void DeferAnimation(int64_t animationTag); - bool HasActiveAnimations() const noexcept; + bool HasStoppedAnimations() const noexcept; + void StopAnimation(int64_t stoppedAnimations); static constexpr std::wstring_view s_valueName{L"v"}; static constexpr std::wstring_view s_offsetName{L"o"}; @@ -57,5 +58,6 @@ class ValueAnimatedNode : public AnimatedNode { std::unordered_set m_dependentPropsNodes{}; std::unordered_set m_activeTrackingNodes{}; std::unordered_set m_deferredAnimations{}; + std::unordered_set m_stoppedAnimations{}; }; } // namespace Microsoft::ReactNative From d826af115be1482c9050d2de81804f83e037f9ed Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 22 Nov 2021 15:01:01 -0500 Subject: [PATCH 05/13] yarn format --- .../Modules/Animated/NativeAnimatedNodeManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 8d1b2a042b1..12341925b52 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -403,7 +403,7 @@ TrackingAnimatedNode *NativeAnimatedNodeManager::GetTrackingAnimatedNode(int64_t return nullptr; } -AnimationDriver* NativeAnimatedNodeManager::GetActiveAnimation(int64_t tag) { +AnimationDriver *NativeAnimatedNodeManager::GetActiveAnimation(int64_t tag) { if (m_activeAnimations.count(tag)) { return m_activeAnimations.at(tag).get(); } From 103d50822e09952aa3b9c56868fb37bfa0374794 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 22 Nov 2021 15:03:15 -0500 Subject: [PATCH 06/13] Start deferred animations after all stopped animations complete, rather than all active animations --- .../Modules/Animated/ValueAnimatedNode.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp index 4a7a59a0a69..cc189edc3ac 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp @@ -96,18 +96,21 @@ void ValueAnimatedNode::AddActiveAnimation(std::shared_ptr anim void ValueAnimatedNode::RemoveActiveAnimation(int64_t animationTag) { m_activeAnimations.erase(animationTag); - m_stoppedAnimations.erase(animationTag); if (!m_activeAnimations.size()) { if (const auto manager = m_manager.lock()) { - // Dispose completed props animations for (const auto &props : m_dependentPropsNodes) { if (const auto propsNode = manager->GetPropsAnimatedNode(props)) propsNode->DisposeCompletedAnimation(Tag()); } + } + } - // Start any deferred animations, if the animation was stopped before - // this is run, we will get a null pointer for the animation driver. We - // also remove any deferred animation tags in StopAnimation. + // Start any deferred animations. If the animation was stopped before this is + // run, we will get a null pointer for the animation driver. We also remove + // any deferred animation tags in StopAnimation. + m_stoppedAnimations.erase(animationTag); + if (!m_stoppedAnimations.size() && m_deferredAnimations.size()) { + if (const auto manager = m_manager.lock()) { for (const auto &deferredId : m_deferredAnimations) { if (const auto animationDriver = manager->GetActiveAnimation(deferredId)) { animationDriver->StartAnimation(); From 3ae29ece9d5f30953cb8e8905a5828e482a2349b Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Thu, 25 Nov 2021 13:32:00 -0500 Subject: [PATCH 07/13] Move completion animations holders to NativeAnimatedNodeManager --- .../Modules/Animated/AnimationDriver.cpp | 31 +++----- .../Animated/NativeAnimatedNodeManager.cpp | 77 ++++++++++++++++--- .../Animated/NativeAnimatedNodeManager.h | 9 ++- .../Modules/Animated/ValueAnimatedNode.cpp | 39 +--------- .../Modules/Animated/ValueAnimatedNode.h | 10 +-- 5 files changed, 85 insertions(+), 81 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index f2604dd1d19..9876ff83f21 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -42,39 +42,27 @@ AnimationDriver::~AnimationDriver() { } void AnimationDriver::StartAnimation() { - auto const animatedValue = GetAnimatedValue(); - // If the animated value has any stopped animations, we must wait for a completion - // callback on these animations to ensure the latest value is correct. - if (animatedValue && animatedValue->HasStoppedAnimations()) { - animatedValue->DeferAnimation(m_id); - return; - } - const auto [animation, scopedBatch] = MakeAnimation(m_config); - if (animatedValue) { + if (auto const animatedValue = GetAnimatedValue()) { animatedValue->PropertySet().StartAnimation(ValueAnimatedNode::s_valueName, animation); - - // The ValueAnimatedNode needs to retain a reference to this animation driver - // so it can be kept alive until the scoped batch completion callback fires. - animatedValue->AddActiveAnimation(shared_from_this()); + animatedValue->AddActiveAnimation(m_id); } scopedBatch.End(); m_scopedBatchCompletedToken = scopedBatch.Completed( [weakSelf = weak_from_this(), weakManager = m_manager, id = m_id, tag = m_animatedValueTag](auto sender, auto) { - if (const auto strongSelf = weakSelf.lock()) { - if (strongSelf->m_ignoreCompletedHandlers) { - return; - } - - strongSelf->DoCallback(strongSelf->m_stopped); - } - if (auto manager = weakManager.lock()) { if (auto const animatedValue = manager->GetValueAnimatedNode(tag)) { animatedValue->RemoveActiveAnimation(id); } manager->RemoveActiveAnimation(id); + manager->RemoveStoppedAnimation(id); + } + + if (const auto strongSelf = weakSelf.lock()) { + if (!strongSelf->m_ignoreCompletedHandlers) { + strongSelf->DoCallback(!strongSelf->m_stopped); + } } }); @@ -84,7 +72,6 @@ void AnimationDriver::StartAnimation() { void AnimationDriver::StopAnimation(bool ignoreCompletedHandlers) { if (const auto animatedValue = GetAnimatedValue()) { - animatedValue->StopAnimation(m_id); animatedValue->PropertySet().StopAnimation(ValueAnimatedNode::s_valueName); m_stopped = true; m_ignoreCompletedHandlers = ignoreCompletedHandlers; diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 12341925b52..131df4b6f7d 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -122,8 +122,26 @@ void NativeAnimatedNodeManager::DisconnectAnimatedNode(int64_t parentNodeTag, in void NativeAnimatedNodeManager::StopAnimation(int64_t animationId) { if (m_activeAnimations.count(animationId)) { - if (const auto animation = m_activeAnimations.at(animationId).get()) { + if (const auto animation = m_activeAnimations.at(animationId)) { animation->StopAnimation(); + + // Insert the animation into the pending completion set to ensure it is + // not destroyed before the callback occurs. It's safe to assume the + // scoped batch completion callback has not run, since if it had, the + // animation would have been removed from the set of active animations. + m_pendingCompletionAnimations.insert({animationId, animation}); + + // Add the animation tag to the set of stopped animations for the value + // node. This is used to ensure animations on this value node do not + // start until all stopped animations fire their completion callbacks. + const auto nodeTag = animation->AnimatedValueTag(); + if (nodeTag != -1) { + if (!m_stoppedAnimations.count(nodeTag)) { + m_stoppedAnimations.insert({nodeTag, {}}); + } + m_stoppedAnimations.at(nodeTag).insert(animationId); + } + m_activeAnimations.erase(animationId); } } @@ -241,13 +259,17 @@ void NativeAnimatedNodeManager::StartAnimatingNode( break; } + // If the animated value node has any stopped animations, defer start until + // all stopped animations fire completion callback and have latest values. if (m_activeAnimations.count(animationId)) { - m_activeAnimations.at(animationId)->StartAnimation(); - - for (auto const &trackingAndLead : m_trackingAndLeadNodeTags) { - if (std::get<1>(trackingAndLead) == animatedNodeTag) { - RestartTrackingAnimatedNode(std::get<0>(trackingAndLead), std::get<1>(trackingAndLead), manager); + if (m_stoppedAnimations.find(animatedNodeTag) != m_stoppedAnimations.end()) { + const auto deferredIter = m_deferredAnimations.find(animatedNodeTag); + if (deferredIter == m_deferredAnimations.end()) { + m_deferredAnimations.insert({animatedNodeTag, {}}); } + m_deferredAnimations.at(animatedNodeTag).insert(animationId); + } else { + StartAnimationAndTrackingNodes(animationId, animatedNodeTag, manager); } } } @@ -403,14 +425,45 @@ TrackingAnimatedNode *NativeAnimatedNodeManager::GetTrackingAnimatedNode(int64_t return nullptr; } -AnimationDriver *NativeAnimatedNodeManager::GetActiveAnimation(int64_t tag) { - if (m_activeAnimations.count(tag)) { - return m_activeAnimations.at(tag).get(); +void NativeAnimatedNodeManager::RemoveActiveAnimation(int64_t tag) { + m_activeAnimations.erase(tag); +} + +void NativeAnimatedNodeManager::RemoveStoppedAnimation(int64_t tag) { + if (m_pendingCompletionAnimations.count(tag)) { + // Remove from stopped animations for value node + const auto animation = m_pendingCompletionAnimations.at(tag); + const auto nodeTag = animation->AnimatedValueTag(); + if (m_stoppedAnimations.count(nodeTag)) { + auto stoppedAnimations = m_stoppedAnimations.at(nodeTag); + stoppedAnimations.erase(tag); + if (stoppedAnimations.size() == 0) { + m_stoppedAnimations.erase(nodeTag); + StartDeferredAnimationsForValueNode(nodeTag); + } + } + m_pendingCompletionAnimations.erase(tag); } - return nullptr; } -void NativeAnimatedNodeManager::RemoveActiveAnimation(int64_t tag) { - m_activeAnimations.erase(tag); +void NativeAnimatedNodeManager::StartDeferredAnimationsForValueNode(int64_t tag) { + if (m_deferredAnimations.count(tag)) { + const auto deferredAnimations = m_deferredAnimations.at(tag); + for (const auto &animationTag : deferredAnimations) { + StartAnimationAndTrackingNodes(animationTag, tag, shared_from_this()); + } + m_deferredAnimations.erase(tag); + } +} + +void NativeAnimatedNodeManager::StartAnimationAndTrackingNodes(int64_t tag, int64_t nodeTag, const std::shared_ptr &manager) { + if (m_activeAnimations.count(tag)) { + m_activeAnimations.at(tag)->StartAnimation(); + for (auto const &trackingAndLead : m_trackingAndLeadNodeTags) { + if (std::get<1>(trackingAndLead) == nodeTag) { + RestartTrackingAnimatedNode(std::get<0>(trackingAndLead), std::get<1>(trackingAndLead), manager); + } + } + } } } // namespace Microsoft::ReactNative diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h index 3344e510cc2..dcd654f33da 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h @@ -37,7 +37,7 @@ class TransformAnimatedNode; class TrackingAnimatedNode; class AnimationDriver; class EventAnimationDriver; -class NativeAnimatedNodeManager { +class NativeAnimatedNodeManager : public std::enable_shared_from_this { public: void CreateAnimatedNode( int64_t tag, @@ -89,8 +89,10 @@ class NativeAnimatedNodeManager { TransformAnimatedNode *GetTransformAnimatedNode(int64_t tag); TrackingAnimatedNode *GetTrackingAnimatedNode(int64_t tag); - AnimationDriver *GetActiveAnimation(int64_t tag); void RemoveActiveAnimation(int64_t tag); + void RemoveStoppedAnimation(int64_t tag); + void StartDeferredAnimationsForValueNode(int64_t valueNodeTag); + void StartAnimationAndTrackingNodes(int64_t tag, int64_t nodeTag, const std::shared_ptr &manager); private: std::unordered_map> m_valueNodes{}; @@ -101,6 +103,9 @@ class NativeAnimatedNodeManager { std::unordered_map, std::vector>> m_eventDrivers{}; std::unordered_map> m_activeAnimations{}; + std::unordered_map> m_pendingCompletionAnimations{}; + std::unordered_map> m_stoppedAnimations{}; + std::unordered_map> m_deferredAnimations{}; std::vector> m_trackingAndLeadNodeTags{}; std::vector m_delayedPropsNodes{}; diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp index cc189edc3ac..f27f42557b7 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.cpp @@ -82,8 +82,8 @@ void ValueAnimatedNode::RemoveDependentPropsNode(int64_t propsNodeTag) { m_dependentPropsNodes.erase(propsNodeTag); } -void ValueAnimatedNode::AddActiveAnimation(std::shared_ptr animation) { - m_activeAnimations.insert({animation->Id(), animation}); +void ValueAnimatedNode::AddActiveAnimation(int64_t animationTag) { + m_activeAnimations.insert(animationTag); if (m_activeAnimations.size() == 1) { if (const auto manager = m_manager.lock()) { for (const auto &props : m_dependentPropsNodes) { @@ -104,21 +104,6 @@ void ValueAnimatedNode::RemoveActiveAnimation(int64_t animationTag) { } } } - - // Start any deferred animations. If the animation was stopped before this is - // run, we will get a null pointer for the animation driver. We also remove - // any deferred animation tags in StopAnimation. - m_stoppedAnimations.erase(animationTag); - if (!m_stoppedAnimations.size() && m_deferredAnimations.size()) { - if (const auto manager = m_manager.lock()) { - for (const auto &deferredId : m_deferredAnimations) { - if (const auto animationDriver = manager->GetActiveAnimation(deferredId)) { - animationDriver->StartAnimation(); - } - } - m_deferredAnimations.clear(); - } - } } void ValueAnimatedNode::AddActiveTrackingNode(int64_t trackingNodeTag) { @@ -129,14 +114,6 @@ void ValueAnimatedNode::RemoveActiveTrackingNode(int64_t trackingNodeTag) { m_activeTrackingNodes.erase(trackingNodeTag); } -bool ValueAnimatedNode::HasStoppedAnimations() const noexcept { - return m_stoppedAnimations.size(); -} - -void ValueAnimatedNode::DeferAnimation(int64_t animationTag) { - m_deferredAnimations.insert(animationTag); -} - void ValueAnimatedNode::UpdateTrackingNodes() { if (auto const manager = m_manager.lock()) { for (auto trackingNodeTag : m_activeTrackingNodes) { @@ -147,16 +124,4 @@ void ValueAnimatedNode::UpdateTrackingNodes() { } } -void ValueAnimatedNode::StopAnimation(int64_t animationTag) { - if (m_deferredAnimations.count(animationTag)) { - // This is not strictly necessary as the animation will be destroyed by the - // NativeAnimatedNodesManager if it was stopped before its deferred start. - m_deferredAnimations.erase(animationTag); - } else { - // The animation should already be running if it is not currently deferred. - assert(m_activeAnimations.count(animationTag)); - m_stoppedAnimations.insert(animationTag); - } -} - } // namespace Microsoft::ReactNative diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h index 7428a14abdc..8b8661fbd4c 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h @@ -32,15 +32,11 @@ class ValueAnimatedNode : public AnimatedNode { void AddDependentPropsNode(int64_t propsNodeTag); void RemoveDependentPropsNode(int64_t propsNodeTag); - void AddActiveAnimation(std::shared_ptr animation); + void AddActiveAnimation(int64_t animationTag); void RemoveActiveAnimation(int64_t animationTag); void AddActiveTrackingNode(int64_t trackingNodeTag); void RemoveActiveTrackingNode(int64_t trackingNodeTag); - void DeferAnimation(int64_t animationTag); - bool HasStoppedAnimations() const noexcept; - void StopAnimation(int64_t stoppedAnimations); - static constexpr std::wstring_view s_valueName{L"v"}; static constexpr std::wstring_view s_offsetName{L"o"}; @@ -54,10 +50,8 @@ class ValueAnimatedNode : public AnimatedNode { private: void UpdateTrackingNodes(); - std::unordered_map> m_activeAnimations{}; + std::unordered_set m_activeAnimations{}; std::unordered_set m_dependentPropsNodes{}; std::unordered_set m_activeTrackingNodes{}; - std::unordered_set m_deferredAnimations{}; - std::unordered_set m_stoppedAnimations{}; }; } // namespace Microsoft::ReactNative From c048ad502ca541b94ba282089902c0adfc10efe6 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Thu, 25 Nov 2021 13:32:28 -0500 Subject: [PATCH 08/13] yarn format --- .../Modules/Animated/NativeAnimatedNodeManager.cpp | 7 +++++-- .../Modules/Animated/NativeAnimatedNodeManager.h | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 131df4b6f7d..3e08ad69f67 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -141,7 +141,7 @@ void NativeAnimatedNodeManager::StopAnimation(int64_t animationId) { } m_stoppedAnimations.at(nodeTag).insert(animationId); } - + m_activeAnimations.erase(animationId); } } @@ -456,7 +456,10 @@ void NativeAnimatedNodeManager::StartDeferredAnimationsForValueNode(int64_t tag) } } -void NativeAnimatedNodeManager::StartAnimationAndTrackingNodes(int64_t tag, int64_t nodeTag, const std::shared_ptr &manager) { +void NativeAnimatedNodeManager::StartAnimationAndTrackingNodes( + int64_t tag, + int64_t nodeTag, + const std::shared_ptr &manager) { if (m_activeAnimations.count(tag)) { m_activeAnimations.at(tag)->StartAnimation(); for (auto const &trackingAndLead : m_trackingAndLeadNodeTags) { diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h index dcd654f33da..d6113cd4829 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h @@ -92,7 +92,10 @@ class NativeAnimatedNodeManager : public std::enable_shared_from_this &manager); + void StartAnimationAndTrackingNodes( + int64_t tag, + int64_t nodeTag, + const std::shared_ptr &manager); private: std::unordered_map> m_valueNodes{}; From 2745b1ca30930c478ea20a4812b361a67a5de44b Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Thu, 2 Dec 2021 16:21:23 -0500 Subject: [PATCH 09/13] Do not remove the active animation from the animated value node for tracking animation nodes, as we skipped that step previously --- .../Modules/Animated/AnimationDriver.cpp | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index 9876ff83f21..2d3d9f06421 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -51,20 +51,22 @@ void AnimationDriver::StartAnimation() { m_scopedBatchCompletedToken = scopedBatch.Completed( [weakSelf = weak_from_this(), weakManager = m_manager, id = m_id, tag = m_animatedValueTag](auto sender, auto) { - if (auto manager = weakManager.lock()) { - if (auto const animatedValue = manager->GetValueAnimatedNode(tag)) { - animatedValue->RemoveActiveAnimation(id); - } - manager->RemoveActiveAnimation(id); - manager->RemoveStoppedAnimation(id); + const auto strongSelf = weakSelf.lock(); + const auto ignoreCompletedHandlers = strongSelf && strongSelf->m_ignoreCompletedHandlers; + if (auto manager = weakManager.lock()) { + if (auto const animatedValue = manager->GetValueAnimatedNode(tag)) { + if (!ignoreCompletedHandlers) { + animatedValue->RemoveActiveAnimation(id); } + } + manager->RemoveActiveAnimation(id); + manager->RemoveStoppedAnimation(id); + } - if (const auto strongSelf = weakSelf.lock()) { - if (!strongSelf->m_ignoreCompletedHandlers) { - strongSelf->DoCallback(!strongSelf->m_stopped); - } - } - }); + if (strongSelf && !ignoreCompletedHandlers) { + strongSelf->DoCallback(!strongSelf->m_stopped); + } + }); m_animation = animation; m_scopedBatch = scopedBatch; From 26d3bdde8b9fd5eec566f8437d6ee7c6db65c521 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Thu, 2 Dec 2021 16:33:37 -0500 Subject: [PATCH 10/13] Do not add enable_shared_from_this to NativeAnimatedNodeManager --- .../Modules/Animated/AnimationDriver.cpp | 28 +++++++++---------- .../Animated/NativeAnimatedNodeManager.cpp | 12 +++++--- .../Animated/NativeAnimatedNodeManager.h | 8 ++++-- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index 2d3d9f06421..8543262a3e6 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -51,22 +51,22 @@ void AnimationDriver::StartAnimation() { m_scopedBatchCompletedToken = scopedBatch.Completed( [weakSelf = weak_from_this(), weakManager = m_manager, id = m_id, tag = m_animatedValueTag](auto sender, auto) { - const auto strongSelf = weakSelf.lock(); - const auto ignoreCompletedHandlers = strongSelf && strongSelf->m_ignoreCompletedHandlers; - if (auto manager = weakManager.lock()) { - if (auto const animatedValue = manager->GetValueAnimatedNode(tag)) { - if (!ignoreCompletedHandlers) { - animatedValue->RemoveActiveAnimation(id); + const auto strongSelf = weakSelf.lock(); + const auto ignoreCompletedHandlers = strongSelf && strongSelf->m_ignoreCompletedHandlers; + if (auto manager = weakManager.lock()) { + if (auto const animatedValue = manager->GetValueAnimatedNode(tag)) { + if (!ignoreCompletedHandlers) { + animatedValue->RemoveActiveAnimation(id); + } + } + manager->RemoveActiveAnimation(id); + manager->RemoveStoppedAnimation(id, manager); } - } - manager->RemoveActiveAnimation(id); - manager->RemoveStoppedAnimation(id); - } - if (strongSelf && !ignoreCompletedHandlers) { - strongSelf->DoCallback(!strongSelf->m_stopped); - } - }); + if (strongSelf && !ignoreCompletedHandlers) { + strongSelf->DoCallback(!strongSelf->m_stopped); + } + }); m_animation = animation; m_scopedBatch = scopedBatch; diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 3e08ad69f67..767187a8912 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -429,7 +429,9 @@ void NativeAnimatedNodeManager::RemoveActiveAnimation(int64_t tag) { m_activeAnimations.erase(tag); } -void NativeAnimatedNodeManager::RemoveStoppedAnimation(int64_t tag) { +void NativeAnimatedNodeManager::RemoveStoppedAnimation( + int64_t tag, + const std::shared_ptr &manager) { if (m_pendingCompletionAnimations.count(tag)) { // Remove from stopped animations for value node const auto animation = m_pendingCompletionAnimations.at(tag); @@ -439,18 +441,20 @@ void NativeAnimatedNodeManager::RemoveStoppedAnimation(int64_t tag) { stoppedAnimations.erase(tag); if (stoppedAnimations.size() == 0) { m_stoppedAnimations.erase(nodeTag); - StartDeferredAnimationsForValueNode(nodeTag); + StartDeferredAnimationsForValueNode(nodeTag, manager); } } m_pendingCompletionAnimations.erase(tag); } } -void NativeAnimatedNodeManager::StartDeferredAnimationsForValueNode(int64_t tag) { +void NativeAnimatedNodeManager::StartDeferredAnimationsForValueNode( + int64_t tag, + const std::shared_ptr &manager) { if (m_deferredAnimations.count(tag)) { const auto deferredAnimations = m_deferredAnimations.at(tag); for (const auto &animationTag : deferredAnimations) { - StartAnimationAndTrackingNodes(animationTag, tag, shared_from_this()); + StartAnimationAndTrackingNodes(animationTag, tag, manager); } m_deferredAnimations.erase(tag); } diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h index d6113cd4829..81c218edbcc 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h @@ -37,7 +37,7 @@ class TransformAnimatedNode; class TrackingAnimatedNode; class AnimationDriver; class EventAnimationDriver; -class NativeAnimatedNodeManager : public std::enable_shared_from_this { +class NativeAnimatedNodeManager { public: void CreateAnimatedNode( int64_t tag, @@ -90,8 +90,10 @@ class NativeAnimatedNodeManager : public std::enable_shared_from_this &manager); + void StartDeferredAnimationsForValueNode( + int64_t valueNodeTag, + const std::shared_ptr &manager); void StartAnimationAndTrackingNodes( int64_t tag, int64_t nodeTag, From d77675869c968d4ea56d2050f3f00c9c4975d65f Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Wed, 15 Dec 2021 20:49:25 -0500 Subject: [PATCH 11/13] Fixes restart for tracking animations Some separate handling needs to be done to effectively restart tracking animated nodes. This change ensures that we first stop the tracking animation (to remove it from the active animations list and put it's value node as a key for deferring animations), and fixes the completion callbacks to avoid removing active animations for tracking nodes, since they use the same animation ID each time the animation is restarted. Fixes #9206 --- .../Modules/Animated/AnimationDriver.cpp | 9 ++++++--- .../Modules/Animated/NativeAnimatedNodeManager.cpp | 4 ++-- .../Modules/Animated/NativeAnimatedNodeManager.h | 2 +- .../Modules/Animated/TrackingAnimatedNode.cpp | 3 +++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index 8543262a3e6..60a996ebb67 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -54,12 +54,15 @@ void AnimationDriver::StartAnimation() { const auto strongSelf = weakSelf.lock(); const auto ignoreCompletedHandlers = strongSelf && strongSelf->m_ignoreCompletedHandlers; if (auto manager = weakManager.lock()) { - if (auto const animatedValue = manager->GetValueAnimatedNode(tag)) { - if (!ignoreCompletedHandlers) { + // If the animation was stopped for a tracking node, do not clean up the active animation state. + if (!ignoreCompletedHandlers) { + if (const auto animatedValue = manager->GetValueAnimatedNode(tag)) { animatedValue->RemoveActiveAnimation(id); } + manager->RemoveActiveAnimation(id); } - manager->RemoveActiveAnimation(id); + + // Always update the stopped animations in case any animations are deferred for the same value. manager->RemoveStoppedAnimation(id, manager); } diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index 767187a8912..bffdade30e6 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -120,10 +120,10 @@ void NativeAnimatedNodeManager::DisconnectAnimatedNode(int64_t parentNodeTag, in } } -void NativeAnimatedNodeManager::StopAnimation(int64_t animationId) { +void NativeAnimatedNodeManager::StopAnimation(int64_t animationId, bool isTrackingAnimation) { if (m_activeAnimations.count(animationId)) { if (const auto animation = m_activeAnimations.at(animationId)) { - animation->StopAnimation(); + animation->StopAnimation(isTrackingAnimation); // Insert the animation into the pending completion set to ensure it is // not destroyed before the callback occurs. It's safe to assume the diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h index 81c218edbcc..f321eb4d9d2 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h @@ -49,7 +49,7 @@ class NativeAnimatedNodeManager { void DisconnectAnimatedNodeToView(int64_t propsNodeTag, int64_t viewTag); void ConnectAnimatedNode(int64_t parentNodeTag, int64_t childNodeTag); void DisconnectAnimatedNode(int64_t parentNodeTag, int64_t childNodeTag); - void StopAnimation(int64_t animationId); + void StopAnimation(int64_t animationId, bool isTrackingAnimation = false); void RestartTrackingAnimatedNode( int64_t animationId, int64_t animatedToValueTag, diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/TrackingAnimatedNode.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/TrackingAnimatedNode.cpp index 9b9073ad3a7..12a0153cd4e 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/TrackingAnimatedNode.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/TrackingAnimatedNode.cpp @@ -27,6 +27,9 @@ void TrackingAnimatedNode::Update() { void TrackingAnimatedNode::StartAnimation() { if (auto const strongManager = m_manager.lock()) { if (auto const toValueNode = strongManager->GetValueAnimatedNode(m_toValueId)) { + // In case the animation is already running, we need to stop it to free up the + // animationId key in the active animations map in the animation manager. + strongManager->StopAnimation(m_animationId, true); toValueNode->AddActiveTrackingNode(m_tag); m_animationConfig.insert(static_cast(s_toValueIdName), toValueNode->Value()); strongManager->StartTrackingAnimatedNode( From fb687fb02253ff4fc4804312c7c7c075ae443e6f Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 10 Jan 2022 12:08:24 -0500 Subject: [PATCH 12/13] Fix bug when deferred animation is stopped. --- .../Modules/Animated/AnimationDriver.cpp | 8 ++- .../Modules/Animated/AnimationDriver.h | 1 + .../Animated/NativeAnimatedNodeManager.cpp | 55 ++++++++++--------- .../Animated/NativeAnimatedNodeManager.h | 4 +- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp index 60a996ebb67..257c40ef6cc 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.cpp @@ -42,6 +42,7 @@ AnimationDriver::~AnimationDriver() { } void AnimationDriver::StartAnimation() { + m_started = true; const auto [animation, scopedBatch] = MakeAnimation(m_config); if (auto const animatedValue = GetAnimatedValue()) { animatedValue->PropertySet().StartAnimation(ValueAnimatedNode::s_valueName, animation); @@ -76,7 +77,12 @@ void AnimationDriver::StartAnimation() { } void AnimationDriver::StopAnimation(bool ignoreCompletedHandlers) { - if (const auto animatedValue = GetAnimatedValue()) { + if (!m_started) { + // The animation may have been deferred and never started. In this case, + // we will never get a scoped batch completion, so we need to fire the + // callback synchronously. + DoCallback(false); + } else if (const auto animatedValue = GetAnimatedValue()) { animatedValue->PropertySet().StopAnimation(ValueAnimatedNode::s_valueName); m_stopped = true; m_ignoreCompletedHandlers = ignoreCompletedHandlers; diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h index 937d4eafeb4..e8055acb8dc 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/AnimationDriver.h @@ -73,6 +73,7 @@ class AnimationDriver : public std::enable_shared_from_this { // auto revoker for scopedBatch.Completed is broken, tracked by internal bug // #22399779 winrt::event_token m_scopedBatchCompletedToken{}; + bool m_started{false}; bool m_stopped{false}; bool m_ignoreCompletedHandlers{false}; }; diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp index bffdade30e6..64775eadfef 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.cpp @@ -131,15 +131,24 @@ void NativeAnimatedNodeManager::StopAnimation(int64_t animationId, bool isTracki // animation would have been removed from the set of active animations. m_pendingCompletionAnimations.insert({animationId, animation}); - // Add the animation tag to the set of stopped animations for the value - // node. This is used to ensure animations on this value node do not - // start until all stopped animations fire their completion callbacks. const auto nodeTag = animation->AnimatedValueTag(); if (nodeTag != -1) { - if (!m_stoppedAnimations.count(nodeTag)) { - m_stoppedAnimations.insert({nodeTag, {}}); + const auto deferredAnimation = m_deferredAnimationForValues.find(nodeTag); + if (deferredAnimation != m_deferredAnimationForValues.end() && deferredAnimation->second == animationId) { + // If the animation is deferred, just remove the deferred animation + // entry as two animations cannot animate the same value concurrently. + m_deferredAnimationForValues.erase(nodeTag); + } else { + // Since only one animation can be active at a time, there shouldn't + // be any stopped animations for the value node if the animation has + // not been deferred. + assert(!m_valuesWithStoppedAnimation.count(nodeTag)); + // In this case, add the value tag to the set of values with stopped + // animations. This is used to optimize the lookup when determining + // if an animation needs to be deferred (rather than iterating over + // the map of pending completion animations). + m_valuesWithStoppedAnimation.insert(nodeTag); } - m_stoppedAnimations.at(nodeTag).insert(animationId); } m_activeAnimations.erase(animationId); @@ -262,12 +271,12 @@ void NativeAnimatedNodeManager::StartAnimatingNode( // If the animated value node has any stopped animations, defer start until // all stopped animations fire completion callback and have latest values. if (m_activeAnimations.count(animationId)) { - if (m_stoppedAnimations.find(animatedNodeTag) != m_stoppedAnimations.end()) { - const auto deferredIter = m_deferredAnimations.find(animatedNodeTag); - if (deferredIter == m_deferredAnimations.end()) { - m_deferredAnimations.insert({animatedNodeTag, {}}); - } - m_deferredAnimations.at(animatedNodeTag).insert(animationId); + if (m_valuesWithStoppedAnimation.count(animatedNodeTag)) { + // Since only one animation can be active per value at a time, there will + // not be any other deferred animations for the value node. + assert(!m_deferredAnimationForValues.count(animatedNodeTag)); + // Add the animation to the deferred animation map for the value tag. + m_deferredAnimationForValues.insert({animatedNodeTag, animationId}); } else { StartAnimationAndTrackingNodes(animationId, animatedNodeTag, manager); } @@ -433,16 +442,12 @@ void NativeAnimatedNodeManager::RemoveStoppedAnimation( int64_t tag, const std::shared_ptr &manager) { if (m_pendingCompletionAnimations.count(tag)) { - // Remove from stopped animations for value node + // Remove stopped animation for value node entry const auto animation = m_pendingCompletionAnimations.at(tag); const auto nodeTag = animation->AnimatedValueTag(); - if (m_stoppedAnimations.count(nodeTag)) { - auto stoppedAnimations = m_stoppedAnimations.at(nodeTag); - stoppedAnimations.erase(tag); - if (stoppedAnimations.size() == 0) { - m_stoppedAnimations.erase(nodeTag); - StartDeferredAnimationsForValueNode(nodeTag, manager); - } + // If the animation was stopped, attempt to start deferred animations. + if (m_valuesWithStoppedAnimation.erase(nodeTag)) { + StartDeferredAnimationsForValueNode(nodeTag, manager); } m_pendingCompletionAnimations.erase(tag); } @@ -451,12 +456,10 @@ void NativeAnimatedNodeManager::RemoveStoppedAnimation( void NativeAnimatedNodeManager::StartDeferredAnimationsForValueNode( int64_t tag, const std::shared_ptr &manager) { - if (m_deferredAnimations.count(tag)) { - const auto deferredAnimations = m_deferredAnimations.at(tag); - for (const auto &animationTag : deferredAnimations) { - StartAnimationAndTrackingNodes(animationTag, tag, manager); - } - m_deferredAnimations.erase(tag); + if (m_deferredAnimationForValues.count(tag)) { + const auto deferredAnimationTag = m_deferredAnimationForValues.at(tag); + StartAnimationAndTrackingNodes(deferredAnimationTag, tag, manager); + m_deferredAnimationForValues.erase(tag); } } diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h index f321eb4d9d2..7f37fbf66cc 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/NativeAnimatedNodeManager.h @@ -109,8 +109,8 @@ class NativeAnimatedNodeManager { m_eventDrivers{}; std::unordered_map> m_activeAnimations{}; std::unordered_map> m_pendingCompletionAnimations{}; - std::unordered_map> m_stoppedAnimations{}; - std::unordered_map> m_deferredAnimations{}; + std::unordered_set m_valuesWithStoppedAnimation{}; + std::unordered_map m_deferredAnimationForValues{}; std::vector> m_trackingAndLeadNodeTags{}; std::vector m_delayedPropsNodes{}; From 5b91b96f6e6ef92ca13d9613ede51abfaeaf8b7b Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Mon, 10 Jan 2022 13:42:53 -0500 Subject: [PATCH 13/13] Reduce diff by reverting unnecessary changes. --- .../Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h index 8b8661fbd4c..b039e6276d7 100644 --- a/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h +++ b/vnext/Microsoft.ReactNative/Modules/Animated/ValueAnimatedNode.h @@ -11,7 +11,6 @@ using namespace comp; } namespace Microsoft::ReactNative { -class AnimationDriver; class ValueAnimatedNode : public AnimatedNode { public: ValueAnimatedNode( @@ -50,8 +49,8 @@ class ValueAnimatedNode : public AnimatedNode { private: void UpdateTrackingNodes(); - std::unordered_set m_activeAnimations{}; std::unordered_set m_dependentPropsNodes{}; + std::unordered_set m_activeAnimations{}; std::unordered_set m_activeTrackingNodes{}; }; } // namespace Microsoft::ReactNative