[Tizen] Ignore AnimationFinishedCallback when we call Animation::Clear() hard + Resol...

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 <eunkiki.hong@samsung.com>

diff --git a/automated-tests/src/dali/utc-Dali-Animation.cpp b/automated-tests/src/dali/utc-Dali-Animation.cpp
index c2705c1..50c93ed 100644 (file)
--- 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<float>();
+  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<float>();
+  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<float>();
+  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<float>();
+  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 3717aa7..04a51ec 100644 (file)
--- 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 (file)
--- 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 c7791f1..5889bd4 100644 (file)
--- 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 ddecbd0..d852836 100644 (file)
--- a/dali/internal/event/animation/animation-playlist.h
+++ b/dali/internal/event/animation/animation-playlist.h
@@ -19,7 +19,7 @@
  */
 
 // INTERNAL INCLUDES
-#include <dali/devel-api/common/map-wrapper.h>
+#include <dali/devel-api/common/set-wrapper.h>
 #include <dali/internal/common/message.h>
 #include <dali/internal/common/ordered-set.h>
 #include <dali/internal/event/common/complete-notification-interface.h>
@@ -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:
-  OrderedSet<Animation, false>        mAnimations; ///< All existing animations (not owned)
-  std::map<Dali::Animation, uint32_t> mPlaylist;   ///< The currently playing animations (owned through handle).
-                                                   ///< Note we can hold same handles multiple, since OnClear can be called after NotifyCompleted.
+  OrderedSet<Animation, false>          mAnimations;        ///< All existing animations (not owned)
+  std::set<Dali::Animation>             mPlaylist;          ///< The currently playing animations (owned through handle).
+  std::set<NotifierInterface::NotifyId> mIgnoredAnimations; ///< The currently cleard animations. We should not send notification at NotifyCompleted.
 };
 
 /**