From 2200ff9e942eaf22ba58d08fe1a53ef71046d18f Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Thu, 30 May 2024 20:18:50 +0900 Subject: [PATCH] Ignore AnimationFinishedCallback when we call Animation::Clear() hard + Resolve bug when we call Play multiple times There was several bugs at AnimationPlayList. 1. Core::ProcessCoreEvents sequence is like this : SceneEvent -> NotificationManager -> Relayout -> Flush. If we call Animation::Clear() at SceneEvent timing, Notfication will send animation finished signal what we must not send. To avoid this system, let we store the cleared animations at AnimationPlayList. Since AnimationFinished callback required at least 1 frame times after flush, we can ignore whole notify events before Flush done. 2. Scene mPlayList keep the reference of Animation whenever we call Play, there were several problems. For example, If we call Play() 2 times and Clear(), that Dali::Animation never be removed. Change-Id: Ie9d5298311b9e5318fecad21b1124c896fcf3a8e Signed-off-by: Eunki, Hong --- automated-tests/src/dali/utc-Dali-Animation.cpp | 198 +++++++++++++++++++++ dali/internal/common/core-impl.cpp | 3 + dali/internal/event/animation/animation-impl.cpp | 5 +- .../event/animation/animation-playlist.cpp | 73 ++++---- dali/internal/event/animation/animation-playlist.h | 18 +- 5 files changed, 255 insertions(+), 42 deletions(-) diff --git a/automated-tests/src/dali/utc-Dali-Animation.cpp b/automated-tests/src/dali/utc-Dali-Animation.cpp index c2705c1..50c93ed 100644 --- a/automated-tests/src/dali/utc-Dali-Animation.cpp +++ b/automated-tests/src/dali/utc-Dali-Animation.cpp @@ -15966,4 +15966,202 @@ int UtcDaliAnimationGetAnimationId(void) DALI_TEST_CHECK(animation.GetAnimationId() == previousId); END_TEST; +} + +int UtcDaliAnimationFinishedNotEmittedAfterClear(void) +{ + tet_infoline("UtcDaliAnimationFinishedNotEmittedAfterClear"); + + TestApplication application; + + auto actor = Actor::New(); + actor.SetProperty(Actor::Property::POSITION, Vector2(100.0f, 100.0f)); + application.GetScene().Add(actor); + + auto animation = Animation::New(1.0f); + const float origY = actor.GetProperty(Actor::Property::POSITION_Y).Get(); + animation.AnimateTo(Property(actor, Actor::Property::POSITION), Vector3(150.0f, origY, 0.0f), TimePeriod(1.0f)); + + bool signalReceived(false); + AnimationFinishCheck finishCheck(signalReceived); + animation.FinishedSignal().Connect(&application, finishCheck); + + animation.Play(); + + application.SendNotification(); + application.Render(500); + // Animation finished. + application.Render(501); + + uint32_t animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + finishCheck.CheckSignalNotReceived(); + + // Send clear. + animation.Clear(); + + application.SendNotification(); + + // Finished signal not emitted. + finishCheck.CheckSignalNotReceived(); + + END_TEST; +} + +int UtcDaliAnimationReferenceCountCheck01(void) +{ + tet_infoline("UtcDaliAnimationReferenceCountCheck01"); + + TestApplication application; + + auto actor = Actor::New(); + actor.SetProperty(Actor::Property::POSITION, Vector2(100.0f, 100.0f)); + application.GetScene().Add(actor); + + auto animation = Animation::New(1.0f); + const float origY = actor.GetProperty(Actor::Property::POSITION_Y).Get(); + animation.AnimateTo(Property(actor, Actor::Property::POSITION), Vector3(150.0f, origY, 0.0f), TimePeriod(1.0f)); + + bool signalReceived(false); + AnimationFinishCheck finishCheck(signalReceived); + animation.FinishedSignal().Connect(&application, finishCheck); + + animation.Play(); + + application.SendNotification(); + application.Render(500); + + uint32_t animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + animation.Reset(); // Remove reference count. + + // Still reference count is 1 since it is animated now. + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + // Animation finished. + application.Render(501); + + // Still reference count is 1 since it is animated now. + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + // Send finished signal, and then dereferenced + application.SendNotification(); + finishCheck.CheckSignalReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 0, TEST_LOCATION); + + END_TEST; +} + +int UtcDaliAnimationReferenceCountCheck02(void) +{ + tet_infoline("UtcDaliAnimationReferenceCountCheck02"); + + TestApplication application; + + auto actor = Actor::New(); + actor.SetProperty(Actor::Property::POSITION, Vector2(100.0f, 100.0f)); + application.GetScene().Add(actor); + + auto animation = Animation::New(1.0f); + const float origY = actor.GetProperty(Actor::Property::POSITION_Y).Get(); + animation.AnimateTo(Property(actor, Actor::Property::POSITION), Vector3(150.0f, origY, 0.0f), TimePeriod(1.0f)); + + bool signalReceived(false); + AnimationFinishCheck finishCheck(signalReceived); + animation.FinishedSignal().Connect(&application, finishCheck); + + animation.Play(); + + application.SendNotification(); + application.Render(500); + + uint32_t animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + // Send stop. + animation.Stop(); + animation.Reset(); // Remove reference count. + + // Still reference count is 1 since it is animated now. + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + // Animation not finished. But we send Stop(). So finished callback should be emitted. + application.SendNotification(); + application.Render(1); + + // Still reference count is 1 since it is animated now. + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + // Send finished signal, and then dereferenced + application.SendNotification(); + finishCheck.CheckSignalReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 0, TEST_LOCATION); + + END_TEST; +} + +int UtcDaliAnimationReferenceCountCheck03(void) +{ + tet_infoline("UtcDaliAnimationReferenceCountCheck03"); + + TestApplication application; + + auto actor = Actor::New(); + actor.SetProperty(Actor::Property::POSITION, Vector2(100.0f, 100.0f)); + application.GetScene().Add(actor); + + auto animation = Animation::New(1.0f); + const float origY = actor.GetProperty(Actor::Property::POSITION_Y).Get(); + animation.AnimateTo(Property(actor, Actor::Property::POSITION), Vector3(150.0f, origY, 0.0f), TimePeriod(1.0f)); + + bool signalReceived(false); + AnimationFinishCheck finishCheck(signalReceived); + animation.FinishedSignal().Connect(&application, finishCheck); + + animation.Play(); + + application.SendNotification(); + application.Render(500); + + uint32_t animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 1, TEST_LOCATION); + + // Send stop and clear. + animation.Stop(); + animation.Clear(); + animation.Reset(); // Remove reference count. + + // Now reference count is 0 since we dont need to keep it's reference anymore. + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 0, TEST_LOCATION); + + // Animation not finished. But we send Clear(). So finished callback should not be emitted. + application.SendNotification(); + application.Render(1); + + // Still reference count is 1 since it is animated now. + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 0, TEST_LOCATION); + + // Finished signal not emitted, and then dereferenced + application.SendNotification(); + finishCheck.CheckSignalNotReceived(); + animationCount = Dali::DevelAnimation::GetAnimationCount(); + DALI_TEST_EQUALS(animationCount, 0, TEST_LOCATION); + + END_TEST; } \ No newline at end of file diff --git a/dali/internal/common/core-impl.cpp b/dali/internal/common/core-impl.cpp index 0b06b44..fbf2598 100644 --- a/dali/internal/common/core-impl.cpp +++ b/dali/internal/common/core-impl.cpp @@ -308,6 +308,9 @@ void Core::ProcessEvents() RelayoutAndFlush(scenes); + // Notify to animation play list that event processing has finished. + mAnimationPlaylist->EventLoopFinished(); + mUpdateManager->EventProcessingFinished(); // Check if the touch or gestures require updates. diff --git a/dali/internal/event/animation/animation-impl.cpp b/dali/internal/event/animation/animation-impl.cpp index 5ef758c..f359180 100644 --- a/dali/internal/event/animation/animation-impl.cpp +++ b/dali/internal/event/animation/animation-impl.cpp @@ -442,7 +442,7 @@ void Animation::Clear() mPlayCalled = false; // Update the current playlist - mPlaylist.OnClear(*this); + mPlaylist.OnClear(*this, true); } void Animation::AnimateBy(Property& target, Property::Value relativeValue) @@ -835,7 +835,8 @@ void Animation::AnimateBetween(Property target, Dali::KeyFrames keyFrames, Alpha bool Animation::HasFinished() { - bool hasFinished(false); + bool hasFinished(false); + const int32_t playedCount(mAnimation->GetPlayedCount()); if(playedCount > mNotificationCount) diff --git a/dali/internal/event/animation/animation-playlist.cpp b/dali/internal/event/animation/animation-playlist.cpp index 120c288..bb99d42 100644 --- a/dali/internal/event/animation/animation-playlist.cpp +++ b/dali/internal/event/animation/animation-playlist.cpp @@ -77,19 +77,10 @@ void AnimationPlaylist::AnimationDestroyed(Animation& animation) void AnimationPlaylist::OnPlay(Animation& animation) { Dali::Animation handle = Dali::Animation(&animation); - auto iter = mPlaylist.lower_bound(handle); - if(iter != mPlaylist.end() && (*iter).first == handle) - { - // Just increase reference count. - ++(iter->second); - } - else - { - mPlaylist.insert(iter, {handle, 1u}); - } + mPlaylist.insert(handle); } -void AnimationPlaylist::OnClear(Animation& animation) +void AnimationPlaylist::OnClear(Animation& animation, bool ignoreRequired) { Dali::Animation handle = Dali::Animation(&animation); auto iter = mPlaylist.find(handle); @@ -97,26 +88,35 @@ void AnimationPlaylist::OnClear(Animation& animation) // Animation might be removed when NotifyCompleted called. if(DALI_LIKELY(iter != mPlaylist.end())) { - // Just decrease reference count. But if reference count is zero, remove it. - if(--(iter->second) == 0u) - { - mPlaylist.erase(iter); - } + mPlaylist.erase(iter); } + + if(ignoreRequired) + { + mIgnoredAnimations.insert(animation.GetAnimationId()); + } +} + +void AnimationPlaylist::EventLoopFinished() +{ + mIgnoredAnimations.clear(); } void AnimationPlaylist::NotifyProgressReached(NotifierInterface::NotifyId notifyId) { Dali::Animation handle; // Will own handle until all emits have been done. - auto* animation = GetEventObject(notifyId); - if(DALI_LIKELY(animation)) + if(DALI_LIKELY(mIgnoredAnimations.find(notifyId) == mIgnoredAnimations.end())) { - // Check if this animation hold inputed scenegraph animation. - DALI_ASSERT_DEBUG(animation->GetSceneObject()->GetNotifyId() == notifyId); + auto* animation = GetEventObject(notifyId); + if(DALI_LIKELY(animation)) + { + // Check if this animation hold inputed scenegraph animation. + DALI_ASSERT_DEBUG(animation->GetSceneObject()->GetNotifyId() == notifyId); - handle = Dali::Animation(animation); - animation->EmitSignalProgressReached(); + handle = Dali::Animation(animation); + animation->EmitSignalProgressReached(); + } } } @@ -132,25 +132,28 @@ void AnimationPlaylist::NotifyCompleted(CompleteNotificationInterface::Parameter #endif DALI_TRACE_BEGIN_WITH_MESSAGE_GENERATOR(gTraceFilter, "DALI_ANIMATION_FINISHED", [&](std::ostringstream& oss) { - oss << "[n:" << notifierIdList.Count() << "]"; + oss << "[n:" << notifierIdList.Count() << ", i:" << mIgnoredAnimations.size() << "]"; }); for(const auto& notifierId : notifierIdList) { - auto* animation = GetEventObject(notifierId); - if(DALI_LIKELY(animation)) + if(DALI_LIKELY(mIgnoredAnimations.find(notifierId) == mIgnoredAnimations.end())) { - // Check if this animation hold inputed scenegraph animation. - DALI_ASSERT_DEBUG(animation->GetSceneObject()->GetNotifyId() == notifierId); - - // Update loop count. And check whether animation was finished or not. - if(animation->HasFinished()) + auto* animation = GetEventObject(notifierId); + if(DALI_LIKELY(animation)) { - finishedAnimations.push_back(Dali::Animation(animation)); - - // The animation may be present in mPlaylist - remove if necessary - // Note that the animation "Finish" signal is emitted after Stop() has been called - OnClear(*animation); + // Check if this animation hold inputed scenegraph animation. + DALI_ASSERT_DEBUG(animation->GetSceneObject()->GetNotifyId() == notifierId); + + // Update loop count. And check whether animation was finished or not. + if(animation->HasFinished()) + { + finishedAnimations.push_back(Dali::Animation(animation)); + + // The animation may be present in mPlaylist - remove if necessary + // Note that the animation "Finish" signal is emitted after Stop() has been called + OnClear(*animation, false); + } } } } diff --git a/dali/internal/event/animation/animation-playlist.h b/dali/internal/event/animation/animation-playlist.h index e27b4bb..1d69ed9 100644 --- a/dali/internal/event/animation/animation-playlist.h +++ b/dali/internal/event/animation/animation-playlist.h @@ -19,7 +19,7 @@ */ // INTERNAL INCLUDES -#include +#include #include #include #include @@ -75,9 +75,17 @@ public: /** * Called when an animation is cleared. + * @param[in] animation The animation that is cleared. + * @param[in] ignoreRequired Whether to ignore the notify for current event loop, or not. * @post The animation will no longer be referenced by AnimationPlaylist. */ - void OnClear(Animation& animation); + void OnClear(Animation& animation, bool ignoreRequired); + + /** + * Notify from core that current event loop finisehd. + * It will clear all ignored animations at OnClear. + */ + void EventLoopFinished(); /** * @brief Notify that an animation has reached a progress marker @@ -119,9 +127,9 @@ private: // from CompleteNotificationInterface void NotifyCompleted(CompleteNotificationInterface::ParameterList notifierList) override; private: - Integration::OrderedSet mAnimations; ///< All existing animations (not owned) - std::map mPlaylist; ///< The currently playing animations (owned through handle). - ///< Note we can hold same handles multiple, since OnClear can be called after NotifyCompleted. + Integration::OrderedSet mAnimations; ///< All existing animations (not owned) + std::set mPlaylist; ///< The currently playing animations (owned through handle). + std::set mIgnoredAnimations; ///< The currently cleard animations. We should not send notification at NotifyCompleted. }; /** -- 2.7.4